Repository navigation
fix(gradient): route on what bngsim says it carries, and keep every saved state seeded the same - #991
Conversation
…aved state seeded the same Four FD-oracle tests in tests/test_gradient_sbml.py passed on bngsim 0.16.0 and failed on bngsim main. Both causes were on PyBNF's side. lanl/bngsim#941: bngsim keeps a parameter an SBML initialAssignment defines (beta = R0*g/N) tracking its inputs, so their own columns already carry the chain through it. The router added beta's column on top, which was right only while bngsim reported it as zero; on main the gradient came back 2x (r0_free, k_src) to 4x (g_free) too large. The router now asks the model which derived parameters it keeps live (param_is_expression) and drops a seed onto a live one, keeping the input's own axis. The SBML preparation no longer writes a recomputed derived value bngsim already holds (a write one ulp off overrode the expression), and a free parameter or condition that writes a parameter the model's own initialAssignment defines is refused. lanl/bngsim#982: after an unlabelled save_concentrations() bngsim seeds no species from the parameters. PyBNF saves at the start of every .net evaluation, at every scan point, at every saveConcentrations() action, whenever an SBML preparation writes a species, and at every multiple-shooting knot, while the router read the seeding off an unconfigured template; S0 in S() S0 got a column of exactly zero. PyBNF now reads the seeding before each such save (off a pristine twin where the model itself can no longer say) and declares it back after the reset and after every later reset to that state; a knot is declared as depending on no parameter, which bngsim before #982 got wrong in the other direction. An SBML preparation whose recomputed initial values bngsim already holds no longer saves at all. Before every fresh-start sensitivity run the run model is compared with what the route assumed (liveness of the derived parameters on a chain, presence of each seed pair) and a mismatch raises with the name. A scored ss_method=>"newton" scan whose fitted parameter seeds a conserved species is refused on a bngsim build whose Newton solve drops that term (lanl/bngsim#704; 0.16.0 is one), decided by a one-model witness. A multiple-shooting lane a knot was saved into is rebuilt before a segment that starts from the model's own state. The SBML backend no longer reports an amount species in a compartment of size other than 1 as unseeded regardless of bngsim: its seed is in the parameter column in PyBNF's units on every supported bngsim, and the filter made the router add the ic term on top of it for a rate constant that also sets an amount (about 2x on 0.16.0). ADR-0153 records the rule.
The first review of #991 found the run-time check refusing a valid configuration, refusals that missed cases, silently wrong results the change had left in place, and an ADR claim that did not hold. - SBML run-time seeding check: a species the evaluation itself wrote (a condition, a scan, a pre-equilibration phase) is exempt, so one condition estimating S0 = x beside another pinning S = 5 runs and matches central differences instead of being refused. - Newton (KINSOL) scans: the lanl/bngsim#704 refusal is decided on the prepared dose point, so a seed set by setConcentration("A()","Atot") is covered. A request for the initial-condition axis of a species a conservation law holds is refused on every build (#992); zero stays only for a species in no law. The integrate-to-steady-state scan keeps a point tensor that carries the initial-condition axis instead of dropping it and reporting "no forward-sensitivity tensor". - A one-model witness decides whether bngsim differentiates through an overridden derived parameter (0.16.0 does, main does not). Where it does, a run that overrides a derived parameter the right-hand side reads and a requested parameter feeds is refused on the .net action loop, scan points, carried scans, the SBML run model and pre-equilibration, and both multiple-shooting backends. - A relative condition on a derived parameter whose inputs are requested is refused at load. - The initialAssignment-target refusal is narrowed to the cases where the assignment overwrites the write; fitting k_used alone is an ordinary fit. - The labelled-snapshot seeding is removed (bngsim refuses a sensitivity run from a restored label on every supported build); a bare-string seed-map entry is read safely; the nested initialAssignment refusal no longer says "Condition sets" for a fitted parameter. - bngsim floor raised to 0.16.0 in pyproject.toml and the CI install. - The reviewer's oracle tests are added unchanged, with new FD-oracle and refusal tests for each item, changelog fragments, and an amended ADR-0153.
First independent review, and what 54c0258 changesThe review ran each finding on bngsim #864 is fixed by this PR on all three builds. The issue's own Regressions in the first commit, now fixed:
Older defects, fixed here:
The reviewer's test file Per the author's runs, 31 gradient and bngsim test files pass on both builds, with identical counts. A re-review of 54c0258 is in progress. Found along the way and filed separately: #993 and #865 (the scalar-path initialAssignment overwrite, unchanged here). |
…fusals
- Each run is compared with its own experiment's route. apply_routings records the columns
each condition reads beside the union request, and every run-time check (SBML live derived
parameters and seeding, .net seeding, overridden derived parameters, the Newton
initial-condition axis and seeding) compares only the running condition's. A condition
that pins k2 = 0.8 beside another that sets k1 = x now runs and matches central
differences on both builds (SBML 25686.10, .net 784.83) instead of being refused.
- A setConcentration seed row is evaluated at the run's own parameter values, inlining only
derived parameters the run model holds live, and keeps a reached seed whose value is zero
at one point. Atot*dose after setParameter("dose",3), or before a scan over dose, no longer
gives a column a third (or 0.6x) of the truth, and Atot*(dose-1) no longer lets a Newton
scan run on a build without lanl/bngsim#704.
- A scored parameter scan of a fitted parameter, or of one a condition binds a fitted
parameter to, is refused at load on .net and SBML.
- The initialAssignment-write refusal covers species targets, relative conditions, scans of
an assignment target and equilibration conditions.
- The gradient path refuses at run time a bngsim older than 0.16.0, or one whose version does
not parse, naming the installed version and the minimum. Scalar fits keep the 0.5.0 API
floor.
- The multiple-shooting .net lane writes only the parameters its model declares, instead of
swallowing every exception from set_param.
- The reviewer's second oracle file is added unchanged, with new tests, changelog fragments
and an amended ADR-0153.
- Multiple shooting widens the request to every carried state after the routings are applied, which cleared the per-condition record, so every segment was compared with the whole union request. The widening now restores the record, with each state's initial-condition column added to every condition. The edition-2 P4 fit (c2: k1 = x, c3: k2 = 0.8) with job_type = ms now converges on SBML and .net on both builds; it stopped on SBML on both and on .net on 0.16.0. A record that does not name the running condition means no scored experiment runs under it, so nothing is compared there. - The initialAssignment-write refusal is decided per scored (action, condition) pair: a write is overwritten only where the same evaluation changes one of its assignment's inputs. A scan or an equilibration condition writing k2 = 2*k1 in the wildtype, beside a condition that sets k1 = x, is honoured and no longer refused. - Each .net run requests only the parameter columns its own condition's route reads (the whole request where that is none), and the carried-scan guard reads the same list. bngsim cannot scan a parameter it differentiates, and a carried scan must request the columns its state carries, so a carried scan of kf in the wildtype, beside a condition that binds kf = x, now runs and matches central differences. - The intervention check reads the configuration's free parameters, not a condition copy's parameter set, so an amount reading a parameter a condition sets to a constant is not refused as reading a fitted one. - The reviewer's third oracle file is added unchanged, with new tests, changelog fragments and an amended ADR-0153.
…ss fails Two tests of the ADR-0148 codegen review expected a wrong gradient (an AssertionError) on any bngsim with the .net codegen path. On such a build PyBNF now refuses the run instead (ADR-0153), so each test asks the same one-model witness PyBNF decides on: where sensitivity_follows_derived_overrides fails it expects the refusal naming k2 and k1, and elsewhere the gradient must match central differences. The direct-bngsim test in test_bngsim_codegen_attach.py keeps its strict xfail, now keyed on that witness rather than on the presence of the .net codegen module.
Closes #864. Closes #992.
Four finite-difference oracle tests in
tests/test_gradient_sbml.pypass with bngsim v0.16.0 and fail with bngsimmain(c966928). A bisect of bngsim found two merges:save_concentrations(), bngsim's model no longer carries its initial-condition seeds, consistently with holding S(0) fixed. PyBNF overwrites initials and saves, so gradients forS0/I0came back 0.In both cases bngsim's columns match central differences of bngsim's own simulation. PyBNF was right on 0.16 by accident, so the fix is PyBNF's. ADR 0153 records the design.
The rule
PyBNF now asks the run model what bngsim does, and assumes nothing, so the same code is right on every bngsim build:
Also:
.netss_method => "newton"scan is refused on a bngsim build without fix(tutorial): teach lesson 25's two minima, and check it from three seeds (#703) #704. On those builds its initial-condition column comes back 0, and a one-model probe detects that, since no version string or feature key distinguishes the builds.Sibling paths
.netstart-of-run sync,resetConcentrations/saveConcentrations.netscan points; parity and Newton steady-state scans.net_run_protocol, threaded steady-state, continuation and bifurcate scans.netconditions.netChecks
Independent oracles: the FD tests compare the assembled gradient with central differences of PyBNF's scalar loss, which runs no sensitivity code. Closed forms are also checked:
e^{-kt},kf/(kf+kr), and 0 for a knot.New
.netoracle for the An invisible character beside a setOption line (a non-breaking space, for example) makes BNG2.pl skip the line while PyBNF applies it #982 sibling:test_net_fd_oracle_seeded_initial_condition_on_the_parameter_axisfails on bngsimmainbefore this change (0.0 against 0.773).Tests changed:
S0's own axis.Both bngsim builds, with a separate codegen cache each, give identical counts. The gradient files:
test_gradient_sbmltest_gradient_routingtest_gradient_assemblytest_gradient_seeding(new)test_shooting_sbmltest_shooting_nettest_shootingSo do 16 backend files, and the slow and recovery tiers of the gradient and shooting files.
Not established:
Tested on macOS x86_64 (Intel), Python 3.12. An independent review found two regressions and five older defects; the second commit (54c0258) addresses them (see the comment below), and a re-review of it is in progress.