Determinism patches for stHash and st_random - #28
Open
glennhickey wants to merge 2 commits into
Open
Conversation
stHash and stSet iteration walked the bucket array, so the order entries came back in was a function of the numeric value of the keys. Most hashes in cactus are keyed on pointers, which makes that order a function of the heap layout: it shifts between runs of the same program on the same input, and everything downstream of the iteration shifts with it. Chain every entry into a doubly-linked list in insertion order and iterate that instead. Insertion order is under the caller's control, so a hash built from a deterministic sequence of inserts now yields its entries the same way on every run. Costs 16 bytes per live entry (32 -> 48 on x86-64). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017E9w7KrbFEfqNRtkam4u1F
st_random drew from rand(), one sequence shared by the whole process. Two threads drawing from it interleave differently on every run, so anything built from those draws -- cactus's reference ordering and its ancestral base calls, both of which break ties randomly inside an OpenMP loop -- came out different each time. Replace it with splitmix64 in thread-local state. st_randomSeed now seeds the calling thread, which lets a parallel loop seed each iteration from the work item and get the same draws however the threads are scheduled. st_random keeps its old 31 bits of resolution: callers were written against values of that granularity, and at least one test depends on them being exactly representable. Unseeded single-threaded use is still reproducible, as it was before (rand() was never seeded from the clock), though the sequence itself is different. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017E9w7KrbFEfqNRtkam4u1F
glennhickey
added a commit
to ComparativeGenomicsToolkit/hal2vg
that referenced
this pull request
Aug 25, 2026
deps/sonLib was pinned at c660699, behind sonLib master. This moves it to cbb2285, which is master (f81bde0, taking in sonLib#27's write-error checking) plus the two commits of ComparativeGenomicsToolkit/sonLib#28 "Determinism patches for stHash and st_random" namely iterating hashes in insertion order and giving the random number generator per-thread state. Both matter here: hash iteration order and a shared RNG are the two things that make a run's output depend on something other than its input, and hal2vg is expected to be reproducible. Note this pins an unmerged PR head. It needs re-pointing at the merge commit once sonLib#28 lands, and if that branch is rebased or squashed first, cbb2285 stops being reachable from any branch and a fresh `git submodule update --init` against it will fail. Full build and `make test-only` (88 tests) pass against it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017JHRYj2XJLaBCZxavo51TX
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.
see ComparativeGenomicsToolkit/cactus#1985