Repository navigation
ode: leave a rule-set parameter at its end-of-run value after a compiled warm run (#1358) - #1359
Merged
wshlavacek merged 2 commits intoOct 30, 2026
Conversation
…led warm run (lanl#1358) A parameter an assignment rule sets is written by the interpreter's evaluate_functions at every right-hand-side call. The compiled right-hand side (cc or MIR) computes the rule in its own func[] and writes nothing back, and run_warm's rows read the functions through fill_row, so after a compiled run with no events or sensitivities get_param("q") read its value at load: 1.0 where an interpreted run read 0.0183. run_warm's write-back now syncs the model at the state the run ends on, as run()'s per-row refresh already leaves it.
… the event fixture test_a_run_through_the_jit_leaves_it_too was the only check of the fix under the MIR JIT, and it ran nowhere. mir.yml runs a fixed list of files and test_sbml_initial_assignment_param_lift.py is not on it; the python-tests legs build without MIR, so it skipped there, and so it does on a local build without -DBNGSIM_ENABLE_MIR=ON. Its replacement is in test_codegen_outputs.py, the file about the compiled output evaluator that skipped the write. The mir-jit leg runs that file under BNGSIM_CODEGEN_JIT=mir and the python-tests legs run it under cc, so one test covers both backends: after a compiled warm run, b := 0.5*S and a := b read 0.5*S(4), not the 5.0 they hold at load. _EVENT_FAR_AWAY is built by a str.replace on AR_DEPENDENT. If the anchor stops matching, the "cold" cases run the warm path and still pass, and run()'s write-back loses its coverage with nothing red. It now asserts the event is in the text, as this file does for its other derived fixture.
wshlavacek
added a commit
to Montana/bngsim
that referenced
this pull request
Oct 31, 2026
One conflict, in test_sbml_initial_assignment_param_lift.py's imports. It is resolved by taking main's copy whole, which drops this branch's strict xfail on test_a_write_after_a_run_does_not_move_it_to_the_rules_current_value. That xfail cited lanl#1358, and lanl#1359 (5e71d2a) fixed lanl#1358. On main, under BNGSIM_CODEGEN_THRESHOLD=1, the branch's copy reports "[XPASS(strict)] lanl#1358: after a compiled run get_param reads a rule-set parameter at its load value" and FAILS. That is the forced run this branch exists to make clean.
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.
Closes #1358.
What was wrong
A parameter that an assignment rule sets, here
q := base*A, is written into its parameter slot by the interpreter'sNetworkModel::evaluate_functionsat every right-hand-side call. So after an interpreted run,get_param("q")reads the rule's value at the end.The compiled right-hand side (cc or MIR) computes the rule in its own
func[]and writes nothing back. On the warm fast path (Impl::run_warm, which takes a run with no events and no sensitivities), the rows read the functions throughfill_row, and nothing else evaluates them, so the slot kept its value from load. On main 9a26f5b, with theAR_DEPENDENTfixture over t = 0–2:get_param("q")after itThe trajectory is unaffected.
The general path (
run(), taken by a run with events or sensitivities) was already right: it refreshes the model at each row it records, which evaluates the functions.The fix
src/cvode_simulator.cpp,run_warm's write-back. After it publishes the final concentrations and time,sync_model_at(final_t, y_data, ns)evaluates the observables, the functions and the parameters they set at the state the run ends on, asrun()'s per-row refresh leaves them:I first added the same call to
run()as well, and measured whether it changed anything. The tests below pass on main'srun()with and without events and sensitivities, compiled and interpreted, so that copy was redundant and is not in this PR.Tests (
test_sbml_initial_assignment_param_lift.py)test_a_run_leaves_the_rules_value_in_the_parameterchecksget_param("q")after the run. It must equal 0.5·[A] at the run's last row to 1e-12, and e⁻⁴ to 1e-5. Six cases cover both paths:warmwarm compiledcoldrun()(an event at t > 100)cold compiledrun()sensitivityrun()(sensitivity_ic=["A"])sensitivity compiledrun()test_a_run_through_the_jit_leaves_it_toodoes the same underBNGSIM_CODEGEN_JIT=mir, assertingcodegen_backend == "mir". It fails on main and skips on a build without MIR.The only difference between main's core and this one is the added line, so the main-core runs above are the mutation check.
With #1350
#1350 marks
test_a_write_after_a_run_does_not_move_it_to_the_rules_current_valueas a strict xfail under a loweredBNGSIM_CODEGEN_THRESHOLD, citing this issue. With this fix, that test passes under the forced threshold, so the strict xfail would report an unexpected pass. Whichever of the two merges second should drop that one mark. The two#1357marks stay.Measured
test_sbml_initial_assignment_param_lift.py: 20 passed in the default configuration, underBNGSIM_CODEGEN_THRESHOLD=1, and underBNGSIM_CODEGEN_JIT=mir BNGSIM_CODEGEN_THRESHOLD=1. tests: declare what a forced-JIT run cannot pass, so the run is a signal (#1217) #1350's marked test is among them.test_build_provenance.pyskips a case whenBNGSIM_ALLOW_STALE_COREis set, as it was for this run.