Repository navigation
Name a fixed species with its $, and say how to undo a moved model before a network-free run - #1431
Merged
Merged
Conversation
… $, and say how to undo a moved model Two rough edges left after #1385 (#1373). A fixed species could not be named by its .net name. The .net loader drops the `$` (`$A()` is the model's `A()`, `@CP::$Sink()` its `@CP::Sink()`, net_file_loader.cpp), so `Model.set_concentration("$A()", 7)` raised "Species '$A()' not found in model". BNG2.pl's setConcentration takes either spelling, on a fixed species or not (`$B()` sets a plain `B()`), checked with BNG 2.9.3. `set_concentration`, `get_concentration` and `declare_ic_sensitivity` now try a name that is no species again without its `$`, at the start or after `@c::`, and keep the model's name for the bookkeeping keyed by it (`_declared_ic_sens`, `_ic_write_log`). The `$` stripper moves to `_model.py`, which `_network_free_params` already imports from. The parity replay's `resolve_species_name` drops the `$` the same way, where a `setConcentration("$X()")` sent a multi-segment replay to its single-segment fallback; no corpus protocol has one yet. A network-free run refused after an SSA, PSA or ODE run on the same model gave the wrong advice. That run writes its end state into the model, so the next nf or rm run starts there (#1373), and a complex an XML written without the network does not seed is refused. The message said to rewrite the XML or run SSA. It now names that run, `model.clone()` for it, and `model.reset()`, which the new test shows lets the run go on. The user guide says the same. Tests fail on main: 8 (the `$` names in set/get/declare, under a compartment prefix, the declared row kept under the model's name, the refusal's advice on nf and rm, the parity resolver). Mutations: no `$` retry (3 fail), bookkeeping under the given name (1), the old advice (2), no exact `$`-less match in the resolver (1, a complex), its key taken on the `$` name (1), declare exact only (1). Full suite before the last review round: 17,278 passed.
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.
Two rough edges left after #1385 (#1373), found while checking what that review left behind.
A fixed species can now be named with its
$The
.netloader drops the$from a fixed species' name ($A()is the model'sA(), and@CP::$Sink()is@CP::Sink()). SoModel.set_concentration("$A()", 7)raisedSpecies '$A()' not found in model, though that is the name the user's.netand BNGL show. BNG2.pl'ssetConcentrationtakes either spelling. Checked with BNG 2.9.3:"$X(a)"and"X(a)"both set the fixed species, and"$A(x)"sets the plainA(x).set_concentration,get_concentrationanddeclare_ic_sensitivitytry a name that is no species again without its$(at the start, or after@c::). The exact name is still tried first. Bookkeeping keyed by species name (_declared_ic_sens,_ic_write_log) uses the model's name.$stripper moves from_network_free_params.pyto_model.py, which_network_free_paramsalready imports from.resolve_species_namedrops the$the same way. Before, asetConcentration("$X()")sent a multi-segment replay to its single-segment fallback. No protocol in the corpus has one yet.The refusal after an SSA run now says how to undo it
An SSA, PSA or ODE run writes its end state into the model, so since #1385 a network-free run on that model starts there. An XML written without the network seeds only the seed species, so a complex that run left behind gets refused. The message said to rewrite the XML or run SSA. It now names that run, suggests
model.clone()for it, and saysmodel.reset()returns to the initial amounts (or to an unlabeledsave_concentrations()). The new test showsreset()lets the run go ahead. The user guide says the same.Tests
set_concentration/get_concentration("$A()")"$B()"on a species that is not fixed setsB(), as in BNG2.pl$name supersedes the declared IC row under the model's name"@CP::$Sink()"onsink_compart.netdeclare_ic_sensitivity({"$A()": ...})stores the row underA()reset(), andreset()lets nf and rm runresolve_species_name("$X(a)"),"@c::$Y()", a$complex$name for no species is still refusedEach new path was also broken on purpose, and at least one test fails for each: no
$retry (3 fail), bookkeeping under the given name (1), the old advice (2), the resolver without its exact$-less match (1, the complex) or with its key taken on the$name (1), anddeclare_ic_sensitivityon the exact name only (1).The full suite passed locally before the last review round: 17,278 passed. A blind review of the whole branch on a different model found no correctness bug. Four small findings from it were fixed in this commit, and the reviewer checked those fixes.