Skip to content

fix: preserve deterministic behavior with connected updates#74

Open
wshlavacek wants to merge 5 commits into
RuleWorld:masterfrom
wshlavacek:bngsim/connectivity-determinism
Open

fix: preserve deterministic behavior with connected updates#74
wshlavacek wants to merge 5 commits into
RuleWorld:masterfrom
wshlavacek:bngsim/connectivity-determinism

Conversation

@wshlavacek
Copy link
Copy Markdown

@wshlavacek wshlavacek commented May 11, 2026

Summary

  • preserve native MoleculeType reaction order in the connected-update fast path while still using the precomputed connectivity matrix
  • broaden connectivity reachability to cover compatible explicit reactant templates, and compatible product templates when the fired rule has reactants
  • fall back from the connected fast path to the full updater when a firing produces indirect products through bonded-neighborhood traversal
  • add direct same-seed connectivity parity regressions for test/tlbr/tlbr.xml and test/testSuite/t3.xml

Why

On current master, seeded NFsim runs can diverge between the full membership updater and the connectivity-limited updater even on the same XML and RNG seed. The connected-update path was still able to differ from the full updater in two important ways:

  • it could mutate membership structures in a different per-MoleculeType reaction order
  • it could disagree with the full updater about which compatible templates and products needed to be revisited

This branch preserves the full updater's effective ordering and broadens the reachable update set where needed, while still falling back to the full updater for indirect-product cases that the connected fast path cannot safely preserve.

In-Repo Repros

These shipped XML fixtures diverge on master between standard mode and -connect, but match on this branch for the same seed:

  • test/tlbr/tlbr.xml
  • test/testSuite/t3.xml
  • test/AN_chemotaxis/an2.xml

Validation

  • build:
    • cmake -S . -B /tmp/nfsim-build-connectivity
    • cmake --build /tmp/nfsim-build-connectivity -j4
  • direct regressions:
    • uv run --with numpy python -m unittest validate.TestIssueRegressions.test_connectivity_preserves_seeded_tlbr_trajectory validate.TestIssueRegressions.test_connectivity_preserves_seeded_local_function_trajectory
  • focused same-seed XML parity sweep:
    • inspected 16 runnable shipped XML fixtures from test/ and models/
    • 0 runnable fixtures still showed standard-vs-connect divergence on this branch
    • the 3 fixtures listed above still diverge on master and now match on this branch
    • excluded test/testSuite/t_dor2.xml, which fails on both master and this branch because standalone NFsim does not support that two-reactant DOR case

@wshlavacek wshlavacek marked this pull request as draft May 11, 2026 21:37
@wshlavacek
Copy link
Copy Markdown
Author

Converting this PR back to draft for now. Local validation found connect-specific output differences that are not covered by the current build-only checks, including a regression on Tutorial_Example and additional deltas in a few feature-coverage models. We are investigating the connected-update path further and will update this PR with validated fixes/results before requesting review again.

@wshlavacek wshlavacek marked this pull request as ready for review May 12, 2026 18:21
@jrfaeder
Copy link
Copy Markdown

@wshlavacek Please resolve conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants