Repository navigation
Seed the Fortran reference so parity runs are reproducible - #234
Closed
neuromechanist wants to merge 3 commits into
Closed
neuromechanist wants to merge 3 commits into
neuromechanist wants to merge 3 commits into
Conversation
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.
Makes the Fortran reference reproducible, which #228 assumed impossible. Stacked on #233.
We already had the fix, just not in the tracked source
sccn/amica PR #54 — portable + seedable
random_seed— has been applied at build time bynative/patch_sources.pysince epic #165, with the trackedamica15.f90deliberately left as a read-only mirror of upstream master. Upstream has not merged #54, so the reference we validate against has been the unseeded one.Per maintainer direction, pamica now carries #54 in its own source rather than waiting: an unseedable reference cannot anchor a parity gate, so mirroring upstream costs more than it buys. The patch script keeps working on an unpatched upstream source and is now normally a no-op.
Measured
Bundled sample, 50 iterations, two runs each:
amica15mac, unseededseed 42seed 42+max_threads 1Seeding pins the initialization; the residual is thread reduction order. Both must be pinned, so
run_fortran_amicanow setsseed(from its ownseedargument, which it previously accepted and ignored) andmax_threads 1as overrides rather than reading them from params.End-to-end through the harness: two reference runs are now bit-identical.
A trap this opened
native/validate_shim.shcallspatch_sources.py --pin-seed. With the source already patched,patch()sees its marker and returns early, so--pin-seedwould have silently done nothing and the shim-vs-mpif90 comparison would have run non-deterministic builds.--pin-seednow edits the file after patching, and fails loudly if its anchor is missing.A bug my own unit tests missed
The first cut wrote
field_dim [30504]— Python list repr — and the Fortran parser aborted at read time. Every unit test passed because they all used hand-picked keys;filesandfield_dimare lists in the realsample_params.json. Caught only by running the harness.There is now a test that feeds the actual
sample_params.jsonthrough and asserts no value keeps list syntax, plus one for space-separated list rendering.Compatibility
The legacy
amica15macremains the default binary and still runs: the Fortran parser has nocase default, so it silently ignores theseedkeyword. Verified.What this unblocks
#228's remaining question was which of four gate designs to adopt given a non-reproducible reference. With the native engine the reference is now bit-reproducible, so a genuine parity gate is available — the choice becomes whether to make the native engine the harness default. That decision is not in this PR.