Skip to content

Fix FP-tolerant metric tests on Python 3.8#117

Closed
kencan7749 wants to merge 1 commit into
KamitaniLab:devfrom
kencan7749:fix-test-metrics-allclose
Closed

Fix FP-tolerant metric tests on Python 3.8#117
kencan7749 wants to merge 1 commit into
KamitaniLab:devfrom
kencan7749:fix-test-metrics-allclose

Conversation

@kencan7749
Copy link
Copy Markdown
Contributor

Summary

This PR fixes a failure in tests/evals/test_metrics.py on the base environment using Python 3.8.

The failure was caused by exact equality checks on floating-point arrays. The pickled expected values differ from the current NumPy/BLAS output by only around 1 ULP (1.1e-162.2e-16), and np.allclose already passes. This indicates floating-point rounding/version drift rather than an algorithmic change.

Changes

  • Replaced 5 self.assertTrue(np.array_equal(...)) checks with np.testing.assert_allclose(...)

  • Updated test_2d and test_2d_nan

  • Used strict tolerances: rtol=1e-12, atol=0

No implementation code or pickle fixtures were changed.

profile_correlation and pattern_correlation outputs differ from the
pickled expected values by ~1 ULP (1.1e-16 to 2.2e-16) on the base
environment (Python 3.8.5), causing np.array_equal to fail. The
algorithm output is unchanged (allclose=True); only the final-bit
rounding differs due to numpy/BLAS version drift since the fixtures
were created.

Replace 5 assertions in test_2d and test_2d_nan with
np.testing.assert_allclose(rtol=1e-12, atol=0). The tolerance absorbs
1 ULP drift while still flagging any genuine implementation change.
pairwise_identification results are bit-exact (rational counts / n-1)
but use the same assertion form for consistency.

No change to bdpy/evals/metrics.py or the pickle fixtures.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ganow
Copy link
Copy Markdown
Contributor

ganow commented May 12, 2026

Since the issue raised in this PR is expected to be resolved by the merge of #112, I am closing this issue. Please let us know if a similar issue occurs even after #112 is merged.

@ganow ganow closed this May 12, 2026
@ganow
Copy link
Copy Markdown
Contributor

ganow commented May 13, 2026

I think this is fixed in #112 now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants