[FIX] compare the robumeta reference values numerically, not byte for… - #138
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #138 +/- ##
=======================================
Coverage 92.18% 92.18%
=======================================
Files 13 13
Lines 1817 1817
=======================================
Hits 1675 1675
Misses 142 142 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… byte The alignment workflow regenerates the pinned reference values from the pinned R image and fails on any git diff. The values are written at full double precision, and robu() reaches them through linear algebra whose last bits depend on which BLAS kernel R's image picks for the CPU it runs on, so two GitHub runners produce files that differ with the R and robumeta versions identical. On the run that prompted this, 84 of 168 values differed by at most 2.6e-16 absolute -- one unit in the last place -- and the job failed on a tree that had not touched the harness or its input. Re-running it on another runner passed. Printing fewer digits instead would have needed no comparison at all, but test_robumeta_alignment.py holds PyMARE to the pin at rtol=1e-10 on purpose, so the pin has to keep the precision that tolerance reads. validation/robumeta/compare_reference.py compares the two files instead: the source block exactly, since that is where a rotted image shows up rather than a wobbly one, and the numbers to 1e-11. That sits below the tolerance the alignment test holds PyMARE to, so a pin this check accepts is still good to the precision that test relies on, and 250x above the 4e-14 that runner-to-runner rounding has been seen to produce. Checked against the artifact from the failing run, which passes, along with nudges of 3e-14 and 5e-12; and against a nudge of 1e-10, a nudge of 1e-6, a bumped robumeta version, a dropped case and a dropped coefficient, none of which do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jdkent
force-pushed
the
ci/robumeta-tolerance
branch
from
August 19, 2026 11:48
546e1c6 to
6e25f55
Compare
jdkent
added a commit
to jdkent/PyMARE
that referenced
this pull request
Aug 19, 2026
Brings in the likelihood-estimator vectorization (neurostuff#137) and the numeric robumeta reference comparison (neurostuff#138). One real conflict, in pymare/tests/test_estimators.py: both sides added imports to the same two lines. Resolved as the union -- Interval and Options from this branch, weighted_least_squares from master. setup.cfg and estimators.py merged textually but were worth checking rather than trusting. setup.cfg correctly kept both edits: this branch's simplified numpy/scipy pins and master's removal of wrapt, which is now genuinely unused. The estimators.py import block likewise took os/os.path/shutil from here alongside master's dropped wrapt and scipy.optimize and its new bounded_scalar_min. The mechanisms StanMetaRegression depends on -- fit_dataset and _dataset_attr_map -- are untouched by master, so the {"groups": "g"} mapping and the corrected comment above it still hold. 406 tests pass, lint clean. The Stan sampling tests were run explicitly to confirm they sample rather than skip, including test_matches_maximum_likelihood_without_groups, which pins the Stan posterior against the likelihood estimator master just rewrote. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
… byte
The alignment workflow regenerates the pinned reference values from the pinned R image and fails on any git diff. The values are written at full double precision, and robu() reaches them through linear algebra whose last bits depend on which BLAS kernel R's image picks for the CPU it runs on, so two GitHub runners produce files that differ with the R and robumeta versions identical. On the run that prompted this, 84 of 168 values differed by at most 2.6e-16 absolute -- one unit in the last place -- and the job failed on a tree that had not touched the harness or its input. Re-running it on another runner passed.
validation/robumeta/compare_reference.py compares the two files instead: the source block exactly, since that is where a rotted image shows up, and the numbers to 1e-9 relative. That tolerance sits five orders above the largest difference rounding has been seen to produce and five below the tolerances the alignment test itself uses, so last-bit noise passes and a real change in what robumeta computes fails. Checked against the run's own artifact, which passes, and against a value nudged by 1e-6, a bumped robumeta version and a missing case, which do not.