Make ColliderCouplingSet iteration deterministic (use DeterministicState, as HGrid already does) - #71
Open
uopoql wants to merge 1 commit into
Open
Conversation
…ate, as HGrid already does) Co-Authored-By: Claude Fable 5 <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
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.
Problem
salva's rapier integration is not deterministic across processes when any collider is coupledto a fluid. Two runs of the same scene, same build, same inputs, single-threaded, produce two
different fluid states — and they do it as a coin flip, not a drift: repeated runs land on one of
exactly two outcomes.
This is visible with
RAYON_NUM_THREADS=1, so it is not the parallel float-reductionnon-determinism that a multi-threaded solver is expected to have. It is a plain ordering
difference, and it makes replay, lockstep, regression-hashing and cross-machine reproduction
impossible for any scene that uses
ColliderCouplingSet.Mechanism
src/integrations/rapier/fluids_pipeline.rs(around line 174):That is
std::collections::HashMapwith the defaultRandomState, whose keys are drawn from theOS at first use — so the hasher differs per process.
ColliderCouplingEntryvalues are theniterated to build boundary particles and to apply the coupling forces back to the colliders, and
floating-point addition is not associative: a different visit order is a different sum, and the
different sum propagates through the pressure solve into a different fluid state.
Nothing here needs a random hasher. The map is keyed by
ColliderHandle, populated by the user'sown
register_couplingcalls, and never exposed to adversarial input, so the DoS resistanceRandomStateexists to provide buys nothing and costs reproducibility.Fix
Use the crate's own deterministic hasher — the one
HGridis already built on.DeterministicStateis defined atsrc/geometry/hgrid.rs(aBuildHasherreturningFnvHasher::with_key(1820)) and is already what keeps the neighbour grid's iteration stable. Thepatch re-exports it from
src/geometry/mod.rsand hands it toColliderCouplingSet::entries:No new dependency, no new type, no new concept: it applies an idiom the crate already established
to the one map that was missed.
entriesis a public field, so this is technically asource-breaking change for anyone who names its type or assigns a
HashMap<_, _>to it directly;constructing through
ColliderCouplingSet::new()and usingregister_coupling/unregister_couplingis unaffected.Evidence
Built from the
0.10.0sources with and without the three lines, everything else — toolchain,crate graph, features, LTO settings — held identical, and driven through the rapier integration
from a host application that hashes the whole fluid state at the end of a fixed-length run.
1. The discriminator: a splash scene where fluid meets a coupled collider, 10 runs,
RAYON_NUM_THREADS=1, final-state hash per run.75860430f339c4aex7,46cc5bf09af5bcfdx375860430f339c4aex10The patched build was then run as two independent trials of 10: 20/20 identical. The surviving
bit-pattern is the stock build's own majority branch, i.e. the patch does not invent a new
trajectory — it pins the existing one to the insertion order.
2. A control build isolates the three lines as the cure. A second build on the same
toolchain with only those three lines reverted to stock reproduces the stock split exactly —
75860430f339c4aex7 /46cc5bf09af5bcfdx3 over 10 runs. Toolchain and build flags aretherefore neutral on the outcome; the patch alone collapses the two futures into one.
3. The physics is unchanged. A drain scene with no coupled collider in contact runs 5/5 at
4bc3ca7c0d2273f1on the patched build — the same hash the stock build produces. The patchremoves a branch; it does not move the fluid.
4. Cost.
msper simulated second, median of 3, splash at 1 thread: control (unpatched, sametoolchain) 265.1, patched 264.8. Within noise — an FNV hash of a
ColliderHandleagainstRandomState's SipHash is, if anything, cheaper.Scope, stated plainly
This fixes one source of cross-process non-determinism. It does not make salva deterministic
under
rayonfan-out: with multiple threads the same patched build still produces a differenthash on every run (5 distinct hashes in 5 runs), which is float-reduction ordering in the parallel
solver and a separate matter entirely. Single-threaded reproducibility, however, goes from
impossible to exact, and that is the configuration in which deterministic fluid is achievable at
all today.
Happy to add a regression test, split the re-export into its own commit, or adjust the field's
visibility story if the public
entriestype change is a concern.