test: make the MGE bitwise sigma-ladder guards portable across CPUs - #551
Merged
Merged
Conversation
The two `*_default_sigma_list_is_bitwise_unchanged` tests added with #549 asserted exact equality between two different numpy code paths: the implementation builds each sigma with a per-element scalar power (`gaussian.sigma = 10 ** log10_sigma_list[i]`, model_util.py:190 and :271) while the tests built their expectation with a vectorised `10 ** np.linspace(...)`. numpy does not guarantee its scalar and SIMD power loops agree bit for bit, so the two disagree by 1 ULP on AVX-512 hardware -- green on GitHub's runners, red on an AVX-512 developer machine. The guard was therefore not portable. Build the expected ladder element by element instead, so both sides take the same numpy path. The comparison stays exact; only the code path producing the expectation changes. `pytest.approx(rel=1e-8)` is still deliberately not used -- the existing docstring reasoning for that is kept verbatim, and both docstrings now record the portability trap so the expectation is not re-vectorised later. Verified on an AVX-512 host with numpy 2.4.6: both tests failed before this change (mask_radius=3.0/20 at index 18, pixel_scales=0.1/10 at index 4) and pass after. Control-tested by perturbing the implementation defaults by a relative 1e-7, which both tests still catch, so the exactness guarantee is intact. Full test_autogalaxy suite: 1004 passed, 3 skipped. Closes #550 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011tJFsEesnF7rZmn2xvfxUe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #550.
What changed
test__mge_model_from__default_sigma_list_is_bitwise_unchangedandtest__mge_point_model_from__default_sigma_list_is_bitwise_unchanged(added with #549)asserted exact equality between two different numpy code paths. The implementation
builds each sigma with a per-element scalar power
(
gaussian.sigma = 10 ** log10_sigma_list[i],autogalaxy/analysis/model_util.py:190and
:271); the tests built their expectation with a vectorised10 ** np.linspace(...).numpy does not guarantee its scalar and SIMD power loops agree bit for bit, so the two
disagree by 1 ULP on AVX-512 hardware — green on GitHub's runners, red on an AVX-512
developer machine. The regression guard was not portable.
This builds the expected ladder element by element, so both sides take the same
numpy path. The comparison stays bitwise; only the code path producing the expectation
changes.
Test-only. No library source is touched.
What was deliberately not done
pytest.approx(rel=1e-8)is still not used. The test docstring rules it out and thereasoning holds — it only fails once the ladder has moved by a relative ~1e-7, already
past the point where the PyAutoFit identifier (
RESOLUTION = 1e-8) changes. Thatreasoning is kept verbatim; both docstrings now additionally carry a
PORTABILITY TRAPnote explaining why the expectation must stay element-wise, so a later tidy-up does not
re-vectorise it.
Verification
The host used for this change is itself AVX-512 (
avx512f/bw/cd/dq/vl/vnni) with numpy2.4.6, so the failure was reproduced, not assumed — both tests fail on the unmodified
tree and pass after.
The reported footprint was narrower than the real one. Drift occurs at:
mge_model_frommask_radius=3.0, total_gaussians=20mge_model_frommask_radius=3.5, total_gaussians=30mge_point_model_frompixel_scales=0.1, total_gaussians=10mge_point_model_frompixel_scales=0.05, total_gaussians=5Both guards were broken, not just the first.
Control test — an element-wise expectation risks becoming vacuous (the test merely
restating the implementation), so this was checked rather than assumed: perturbing the
implementation defaults by a relative 1e-7 (
sigma_min1e-4→1.0000001e-4,0.01→0.010000001) still fails both tests. The exactness guarantee survives thechange. The implementation was restored afterwards; only the test file is modified here.
Also verified:
np.log10(1e-4) == -4.0andnp.log10(0.01) == -2.0exactly, so thetests' literal endpoints remain a faithful stand-in for the implementation's
np.log10(sigma_min).Suites:
test_autogalaxy/analysis/test_model_util.py29 passed; fulltest_autogalaxy/1004 passed, 3 skipped. Run locally on Python 3.11 (the onlyinterpreter with a stack in that sandbox) — CI grades 3.12/3.13, and the change is
pure-Python test code with no version-sensitive surface.
Scope
Test-only, so no downstream workspace impact and no
pending-releasegate. Theneighbouring
pytest.approx(..., 1.0e-8)assertions (L129, L237) are tolerance-based bydesign and are untouched. PyAutoLens has no equivalent exact-equality
10 ** np.linspaceassertion.🤖 Generated with Claude Code
https://claude.ai/code/session_011tJFsEesnF7rZmn2xvfxUe
Generated by Claude Code