Codecov Report

Patch and project coverage have no change.

Comparison is base (3800c46) 85.92% compared to head (dbbf189) 85.92%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff            @@
##           develop     #286   +/-   ##
========================================
  Coverage    85.92%   85.92%           
========================================
  Files           23       23           
  Lines         6082     6082           
========================================
  Hits          5226     5226           
  Misses         856      856           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@lan496 @atztogo This is weird see 14b1a6b and 8fc8939. If we have the build in Debug mode (as it should be for the CI maybe, or maybe RelWithDebInfo), than the tests succeed. But in the release build, the MacOS test fails at test_magnetic_symmetry.test_with_broken_symmetry. Any clues of what can happen there?

test_magnetic_symmetry.test_with_broken_symmetry is a sensitive case, and may be not very logical to fix it. I would like @lan496 to have a look when you have time.

Yes, this test case is very sensitive to floating-point arithmetic.
In fact, linux and mac(intel) failed to search symmetry, but mac(arm64) returned a symmetry when I checked.

I believe there is a way to write tests against floating point errors so that we test against multiple random values

I do not come up with any way other than checking that the implementation does not cause a segmentation fault.

I've found a quick fix in the meantime, marking the tests as xpass(strict=false) both on C and python side

For the proper solution I think I've found a design, but I need help on the numbers.

Basically we run the tests n times with a random noise. As soon as one set of variable triggers the test failure, we check the error code and everything and due the proper assertions.

The help I need is with: what is the central value that should cause broken_symmetry and what are the suggested noise to apply?

You mean to add some random values to double position[][3]? I have no idea the magnitude of the random noise...

If we run the tests with random noises, what could we guarantee from the test? At least, the xpass(strict=false) workaround checks that the current implementation does not cause a segmentation fault for the difficult situation. I think that it is enough for now.

If we run the tests with random noises, what could we guarantee from the test?

Well the whole point of test_magnetic_symmetry.test_with_broken_symmetry is to test that in numerically unstable regions:

• The calculation fails with the correct expected error
• All of memory allocation/de-allocation is safe around that function call
• The calculation is not passing when it shouldn't

The fact that this test fails (rather that it doesn't fail) is a genuine issue. Spglib is giving a confident answer of the symmetry group when in principle it should be undetermined. Ideally that test should always pass (i.e. it should always give an error) regardless of numerical error, but as a first step it should be confirmed that the compiler does indeed fail for some points, and that the error mechanism is not broken in that compiler.

That's why I see that a gradual way of fixing this is:

1. Test with n random noise and require that at least one of them fails
2. Change the tests to xfail and investigate what's the general behaviour. Technically gtest and catch do not have xfail implemented, but maybe that can be solved in the meantime
3. Resolve the core issue, maybe doing some pre-checks and be more generous in the failure. It should be that for reasonable random variance well within symprec, any test would fail. Then convert xfail to normal assertion.