1

I'm trying to validate matrix values in nested loop for unit tests. The tests pass on macOS (clang) but fail Ubuntu 18.04 (gcc version 7.3.0).

Here what I'm trying to do (I'm trying to validate the result (m3) is identity matrix):

mat3  m1 = GLM_MAT3_IDENTITY_INIT;
mat3  m2 = GLM_MAT3_IDENTITY_INIT;
mat3  m3;

int i, j, k;

/* test identity matrix multiplication */
glm_mat3_mul(m1, m2, m3);

for (i = 0; i < 3; i++) {
  for (j = 0; j < 3; j++) {
    if (i == j) {
      assert_true(glm_eq(m3[i][j], 1.0f));
    } else {
      assert_true(glm_eq(m3[i][j], 0.0f));
    }
  }
}

This fails on Ubuntu/gcc but passes on macOS. If I use volatile on i variable then it passes on Ubuntu too:

mat3  m1 = GLM_MAT3_IDENTITY_INIT;
mat3  m2 = GLM_MAT3_IDENTITY_INIT;
mat3  m3;

volatile i;
int      j, k;

/* test identity matrix multiplication */
glm_mat3_mul(m1, m2, m3);

for (i = 0; i < 3; i++) {
  for (j = 0; j < 3; j++) {
    if (i == j) {
      assert_true(glm_eq(m3[i][j], 1.0f));
    } else {
      assert_true(glm_eq(m3[i][j], 0.0f));
    }
  }
}

if I print i and j values before if (i == j) { statement without volatile then it also passes. Maybe i is cached and new value of i is not loaded at next iteration. But this would also be true for mat4 tests, same test works for mat4

Using one condition (true condition or false condition) causes test to pass but using if with else causes test to fail.

I'm using cmocka for testing and run make check command to run tests.

mat3 and mat4 definitions:

typedef float                   vec2[2];
typedef CGLM_ALIGN_IF(8)  float vec3[3];
typedef int                    ivec3[3];
typedef CGLM_ALIGN_IF(16) float vec4[4];

#ifdef __AVX__
typedef CGLM_ALIGN_IF(32) vec3  mat3[3];
typedef CGLM_ALIGN_IF(32) vec4  mat4[4];
#else
typedef                   vec3  mat3[3];
typedef CGLM_ALIGN_IF(16) vec4  mat4[4];
#endif

Should I use volatile here? Or what is wrong with my test codes?

PS: Same test passes for mat4 on Ubuntu, it only fails for mat3

EDIT:

Float values are compared using glm_eq(a, b):

CGLM_INLINE
bool
glm_eq(float a, float b) {
  return fabsf(a - b) <= FLT_EPSILON;
}
recp
  • 383
  • 1
  • 2
  • 14
  • Are you comparing floats which are the result of calculations for identity to 1.0? That is often trouble. See https://stackoverflow.com/questions/588004/is-floating-point-math-broken – Yunnosch Dec 24 '18 at 08:08
  • I'm comparing floats with [glm_eq()](https://github.com/recp/cglm/blob/b9021978cb1628de4f5b15d4ff4dc08482e2f8df/include/cglm/util.h#L182-L185) – recp Dec 24 '18 at 08:10
  • Does it take the problematic float attributes into account? – Yunnosch Dec 24 '18 at 08:12
  • What it does is `return fabsf(a - b) <= FLT_EPSILON`. Isn't that enough? Also the main problem seems related to loop variables, not about floats, I guess :S – recp Dec 24 '18 at 08:17
  • Please [edit] your question to add the useful information from your comment. – Yunnosch Dec 24 '18 at 08:18
  • @recp: `return fabsf(a - b) <= FLT_EPSILON` is incredibly broken (will only work when all values are tiny). Imagine it as something vaguely like `100000000 * 10**99 - 999999999 * 10**99) <= 1 * 10**(-99)` (where `100000000 * 10**99` and `999999999 * 10**90` were supposed to be equal but aren't due to rounding errors). Note that changing anything (e.g. changing a loop variable to `volatile`) can completely change the resulting assembly language and (e.g.) cause intermediate results of a floating point to use (e.g.) 80-bit floating point instead of 32-bit floating point. – Brendan Dec 24 '18 at 08:33
  • Any better solution to compare two float variables? Should I use bigger value than FLT_EPSILON? Or completely different method to compare? Do you think my tests fail due to float comparisons? I was thought that it uses 80-bit as default (I'm not sure) but mat4 tests are also use same things. Why it fails for mat3 tests, all operations seem same except array size and alignment. I'm just comparing 0.0f and 1.0f not bigger values. – recp Dec 24 '18 at 08:45
  • @recp: I don't know the rules of C well enough. I'd probably try something like `fabs(a/b) <= FLT_EPSILON` and then get worried about overflows/infinity, then get mad, then cast the suckers to `uint32_t` and get more annoyed when I can't do "signed = unsigned - unsigned", then delete it all and use assembly language. :-) – Brendan Dec 24 '18 at 08:49
  • This passes for me: `assert(glm_eq(100000000.0f * 10.0f * 99.0f, 99999999.0f * 10.0f * 99.0f) == true)`. I had to remove one 9 from your expression. – recp Dec 24 '18 at 08:54
  • @recp: Note that there's 2 problems. The first is that the magnitude of the error depends on the magnitude of the values (e.g. for "+/- 2% of 5 million" the error is in the thousands, but for "+/- 2% of 10" the error is a small fraction). The second is that the magnitude of the error increases (by a little or a lot) for every operator used (you need to analyse each step of the entire calculation to determine the worst case error). – Brendan Dec 24 '18 at 09:08
  • @recp: Erm. C doesn't have an "exponentiation operator" so I cheated and used `**`, and the example was supposed to be interpreted more like `100000000.0f * pow(10.0f, 99.0f)`. However the example wasn't meant to be taken literally, but was a "decimal approximation" of what happens in binary floating point numbers (which have the same "significand * pow(base, exponent)" form, just with 2 as the base and not 10). – Brendan Dec 24 '18 at 09:13
  • @recp: For a (more literal) binary example consider `111111111b << 11111111b - 111111110b << 11111111b` where the magnitude of the error is around `1b << 11111111b` (which is a massive number, and not a tiny number like `FLT_EPSILON`) – Brendan Dec 24 '18 at 09:15
  • @Brendan I would like to improve `glm_eq()` function but not sure how :( Using bigger number than epsilon seems not a good solution I guess. I'll compare integer values in same loop to see what will happen (to see why loop fails). – recp Dec 24 '18 at 09:19
  • same loop with integers is passed asserts, and if `glm_eq()` returns 1 then it also passes for float. It seems I need to fix `glm_eq()` – recp Dec 24 '18 at 09:30
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/185712/discussion-between-brendan-and-recp). – Brendan Dec 24 '18 at 09:37

0 Answers0