Skip to content

Commit 20b8a01

Browse files
imikejacksonclaude
andauthored
BUG: Fix bugs in Matrix3X1, Stereographic, and OrientationConverterTest (#37)
- Matrix3X1::normalize() (static): Add missing `return true` on success path (undefined behavior) - Matrix3X1::sortAscending(): Fix `this` to `*this` for copy construction and add missing `return outMat` - Stereographic::isValid(): Fix epsilon from 1.0E15 to 1.0E-15 (validation was effectively disabled) - OrientationConverterTest: Fix tolerance from 1.0E6 to 1.0E-6 and compare via orientation matrices to handle Euler angle gimbal lock ambiguity Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 25f7699 commit 20b8a01

File tree

7 files changed

+376
-16
lines changed

7 files changed

+376
-16
lines changed

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ option(EbsdLib_BUILD_H5SUPPORT "Build H5Support Library" OFF)
3737

3838

3939
# set project's name
40-
project(EbsdLibProj VERSION 2.1.0)
40+
project(EbsdLibProj VERSION 2.2.0)
4141

4242

4343
# Request C++17 standard, using new CMake variables.

Code_Review/TODO.md

Lines changed: 355 additions & 0 deletions
Large diffs are not rendered by default.

Source/EbsdLib/Math/Matrix3X1.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ class Matrix3X1
187187
i = i / denom;
188188
j = j / denom;
189189
k = k / denom;
190+
return true;
190191
}
191192

192193
/**
@@ -195,7 +196,7 @@ class Matrix3X1
195196
*/
196197
SelfType sortAscending()
197198
{
198-
SelfType outMat = this; // copy constructor
199+
SelfType outMat = *this; // copy constructor
199200
T temp;
200201

201202
if(outMat[0] <= outMat[1] && outMat[0] <= outMat[2])
@@ -248,6 +249,7 @@ class Matrix3X1
248249
outMat[2] = temp;
249250
}
250251
}
252+
return outMat;
251253
}
252254

253255
/**

Source/EbsdLib/Orientation/Stereographic.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class Stereographic : public OrientationBase<T, 3>
8585
ResultType res;
8686
res.result = 1;
8787

88-
value_type epsd = 1.0E15; // std::numeric_limits<float>::epsilon();
88+
value_type epsd = 1.0E-15; // std::numeric_limits<float>::epsilon();
8989
ValueType rd = magnitude();
9090
if(rd > 1.0 + epsd)
9191
{

Source/Test/OrientationConverterTest.cpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */
3535
#include <catch2/catch.hpp>
3636

37+
#include <cmath>
3738
#include <cstdio>
3839
#include <iomanip>
3940
#include <iostream>
@@ -42,6 +43,7 @@
4243
#include "EbsdLib/Core/EbsdLibConstants.h"
4344
#include "EbsdLib/EbsdLib.h"
4445
#include "EbsdLib/LaueOps/CubicOps.h"
46+
#include "EbsdLib/Orientation/Euler.hpp"
4547
#include "EbsdLib/Orientation/Quaternion.hpp"
4648
#include "EbsdLib/OrientationMath/OrientationConverter.hpp"
4749

@@ -82,7 +84,7 @@ TEST_CASE("ebsdlib::OrientationConverterTest::TestEuler2Quaternion", "[EbsdLib][
8284
for(size_t i = 0; i < 8; i++)
8385
{
8486
float delta = std::abs(exemplar[i] - (*output)[i]);
85-
REQUIRE(delta < 1.0E6);
87+
REQUIRE(delta < 1.0E-6);
8688
}
8789
}
8890

@@ -132,17 +134,22 @@ void TestEulerAngle(float phi1, float phi, float phi2)
132134
converters[t1]->convertRepresentationTo(ocTypes[t0]);
133135
ebsdlib::FloatArrayType::Pointer t0_output = converters[t1]->getOutputData();
134136

135-
qStride = strides[t0];
136-
std::vector<float> delta(qStride, 0);
137+
// Compare via orientation matrices to avoid Euler angle ambiguities
138+
// (periodicity and gimbal lock at Phi=0 or Phi=pi)
137139
for(size_t i = 0; i < nTuples; i++)
138140
{
139-
float* orig = eulers->getPointer(i * qStride);
140-
float* converted = t0_output->getPointer(i * qStride);
141-
// printf("%s -> %s -> %s\n", tStrings[t0].toLatin1().constData(), tStrings[t1].toLatin1().constData(), tStrings[t0].toLatin1().constData());
142-
for(size_t j = 0; j < qStride; j++)
141+
float* orig = eulers->getPointer(i * 3);
142+
float* converted = t0_output->getPointer(i * 3);
143+
144+
ebsdlib::Euler<float> origEu(orig[0], orig[1], orig[2]);
145+
ebsdlib::Euler<float> convEu(converted[0], converted[1], converted[2]);
146+
auto origOm = origEu.toOrientationMatrix();
147+
auto convOm = convEu.toOrientationMatrix();
148+
149+
for(size_t j = 0; j < 9; j++)
143150
{
144-
delta[j] = std::abs(orig[j] - converted[j]);
145-
REQUIRE(delta[j] < 1.0E6);
151+
float delta = std::abs(origOm[j] - convOm[j]);
152+
REQUIRE(delta < 1.0E-4);
146153
}
147154
}
148155
}

Testing/Temporary/CTestCostData.txt

Lines changed: 0 additions & 1 deletion
This file was deleted.

Testing/Temporary/LastTest.log

Lines changed: 0 additions & 3 deletions
This file was deleted.

0 commit comments

Comments
 (0)