Repository navigation
nfsim: carry #1419's shim check, and run the vendor tripwire in CI - #1447
Merged
Merged
Conversation
#1419 added the bracket-after-call refusal to NFsim's mu::Parser shim (nfsim_funcparser.h) as a direct edit to third_party/nfsim/. A refresh replaces that tree with what upstream plus scripts/nfsim_vendor_patches/ produce, so the next one would have dropped the check without a word: the 20-patch candidate's export differs from main's tree in that file alone. It is now carry 0021, bngsim/carry-shim-bracket-after-call, and the re-vendored tree is byte-identical to main's. It got in because nothing ran the tripwire. NFSIM_VENDORING.md said a paused vendor_verify job in build.yml would, but no such job exists; the check ran only in a manual pre-commit stage. lint.yml gains an nfsim-vendor job that replays the patch series onto the base VENDOR.json records and runs vendor_nfsim.py --verify-clean, about a minute. On main's tree it fails, naming nfsim_funcparser.h.
wshlavacek
added a commit
to Montana/bngsim
that referenced
this pull request
Nov 2, 2026
…al (lanl#1179) NFsim read a table function indexed by time only at its x values and after each reaction (lanl#892) and held that value in between. A linear table moves in between, so where little fired the run lagged the ramp: from 100 X(p~1) leaving at a rate that ramps from 0 at t = 0 to 10 at t = 1, nf kept all 100 through t = 1, where the exact mean is 1.75. method="nf" refused such a model instead (unreleased). Carry 0020 is amended. Where a linear table indexed by time ramps, stepTo() places the next firing where the propensity integrated from the current time reaches an Exp(1) draw, the time-change theorem, and picks the reaction from the propensities there. Between firings the state is fixed, so the propensity moves with the clock alone; it is integrated over error-controlled 5-point Gauss-Lobatto panels, the scheme bngsim's network SSA and RuleMonkey use, each node a propensity refresh with the clock moved there. No panel spans a breakpoint, and a panel ending on one reads an ulp short of it, so a step table that jumps there is read on the side being integrated. A call that stops inside a ramp leaves the rest of its draw to the next; a breakpoint drops it, as it drops a waiting time. Outside every linear table's ramp stepTo() runs as before. Re-exported and re-vendored on top of lanl#1447's carry 0021; --verify-clean --strict passes with 21 patches. The refusal (refuse_nfsim_linear_time_tables and its scan) is removed, with its unreleased changelog entry. The cold start above now reads 73.3, 29.9 and 1.82 at t = 0.25, 0.5 and 1 against exact 73.4, 29.6 and 1.75 (200 seeds, within 1 SE). On this two-rule model a firing costs about what it does with a step table. Tests check against an ODE solve of one molecule's probability: the four starts of the step-table test on outputs at and between the x values, a file table listed in each order, a saturating rate law that reads the table through another function, a step table that jumps inside the ramp, and a session run in segments that end inside it. With the ramp branch switched off, 11 of these 13 fail; the two that pass are the fixture as written, which fires often enough that the held value was about 3 SE off. test_nfsim.py runs the two linear fixtures again and checks that a file table reads its method. The Windows NFsim cluster goes to 329; its floor moves to 321.
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.
#1419 added the bracket-after-call refusal (#1415) to NFsim's mu::Parser shim,
third_party/nfsim/src/NFfunction/nfsim_funcparser.h, as a direct edit to the vendored tree.vendor_nfsim.pyreplaces that tree with what upstream NFsim plusscripts/nfsim_vendor_patches/produce, so the next refresh would have dropped the check without a word. Onmain, a candidate built from the 20 patches differs from the tree in that one file.The carry. The edit is now patch 0021, topic
bngsim/carry-shim-bracket-after-call, listed inCARRY_QUEUEandVENDOR.json. Re-vendored from it,third_party/nfsim/is byte-identical tomain's; onlyVENDOR.jsonchanges.vendor_nfsim.py --verify-clean --strictpasses with 21 patches.Why it got in. Nothing in CI ran the tripwire.
NFSIM_VENDORING.mdsaid a pausedvendor_verifyjob inbuild.ymlwould run it, but there is no such job; the check ran only in a manual pre-commit stage.lint.ymlgains annfsim-vendorjob. It replays the patch series onto thebase_committhatVENDOR.jsonrecords and runs--verify-cleanagainst that candidate. It takes seconds (9 s on its first run), so it has no paths filter, like the pre-commit job beside it. I ran its steps locally: they pass on this branch, and onmain's tree the same comparison flagsnfsim_funcparser.h.This blocks #1179: that fix re-vendors NFsim, which without this carry would have reverted #1419.
Testing. The vendoring and CI meta-tests pass locally (130). Pushed with
--no-verify, since the vendored source is unchanged and CI runs the full suite. No changelog entry: users see no change, so this carries thechangelog exemptlabel.