Repository navigation
Conversation
The default scalar atol=1e-8 is BNG2.pl's, for molecule counts. A species that stays below it all run has no error control but the tolerance: the issue's binding model ended at A = -8.7e-10 for 4.1e-10, and BIOMD0000000002's complexes were off by 1e-2, with no word. run() now raises bngsim.AtolScaleWarning, naming the species, when the caller gave no atol and a species stays below it while reaching at least 1e-6 of the run's largest species. The default is unchanged.
This branch has not been deployed
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 #1361.
What was wrong
The default ODE tolerances are
rtol=1e-8and a scalaratol=1e-8: BNG2.pl's, chosen for species held as molecule counts. A species that stays belowatolfor a whole run has its error controlled by nothing but the tolerance. Models in molar units (SBML, BioModels) are full of such species, and nothing said so. On main c0877fa:tests/data/BIOMD0000000002.xml(L at 1e-5 M; complexes at 3e-10 to 7e-9 M): errors of 1e-2. Its largest species is aboveatol, so a test of the whole run againstatolpasses it. The species that matter are below it.The fix: a warning, not a new default
Simulator.run()raises the newbngsim.AtolScaleWarning(aUserWarning, exported) for an ODE run when both hold:atol: neitherrun(atol=...)norset_tolerances(atol=...), and no vector or trackingatolis in force.atolfor the whole run while reaching at least 1e-6 of the run's largest species. That floor is what keeps a species stuck at rounding in a count model (1e-17 of the rest) quiet, while it catches BIOMD0000000002, whose complexes reach 7e-4 of its ligand.The message names up to five such species and their peak, and names the remedy:
atol="auto"(#196), or a scalaratolwell below them.docs/user-guide/solvers.mdgets a section on the default tolerances and models in small units.Why not change the default?
atol="auto"as the default is the issue's first suggestion, and arguably the better end state. But it changes every default ODE trajectory, BNG2.pl parity at defaults included, so it is a maintainer's call. This PR makes the hazard visible without moving any number; a follow-up can change the default.Measured, for false positives
.netin the repo (tests/data,benchmarks,parity_checks; counts, by BNG2.pl)nanomolar_dimer.net(species at 1.5e-9) andjac_selfcheck_amount_scaled.net(at 1.7e-10). Both are small-unit models, which is what the warning is fortests/data/BIOMD*.xmlAre the 35 right? For each warned BioModel, the default run was compared with a reference at
rtol=1e-12, atol=1e-20. The error is the largest over species, relative to each species' peak:atol="auto", the remedy the warning names: median 6.1e-7.autoas well, so something beyond the tolerance is wrong in them. They are not this PR's.So no warned model was a false alarm.
Tests (
test_atol_scale_warning.py, 8)test_a_molar_model_at_the_default_atol_is_warned: ends negative, one warning naming'A()'andatol='auto'test_a_mixed_scale_biomodel_is_warned: BIOMD0000000002, naming'BL'test_the_remedy_it_names_is_right_and_quiet: control.atol='auto'is within 1e-5 of the reference, with no warningtest_an_atol_the_caller_chose_is_not_second_guessed[run/set_tolerances]: controlstest_a_count_model_is_not_warned: controltest_a_species_at_the_noise_of_a_count_model_is_not_warned: control. A species belowatolbut at 1e-15 of the largesttest_ssa_is_not_warned: control. A concentration of 1e-9 in a compartment of 1e10, ten molecules: SSA has noatolAtolScaleWarningon main)Mutations
set_tolerancesatol is warned as well[set_tolerances]test_ssa_is_not_warnedatol(the first design)Measured
Full suite (macOS arm64, main c0877fa + this branch): 16,424 passed, 0 failed, 137 skipped. Six existing tests now raise the warning, and each is a real case: five run a model whose only species is tiny (7.5e-14 to 3e-9; literal-parsing and rounding tests), and one runs a molar BioModel whose
Bfa1P4_Tem1GDPpeaks at 3.2e-10 while its largest species reaches 1.7e-4. None of them asserts on warnings.pre-commit run --all-files(new files staged) andsphinx -Wwithdocs/requirements-ci.txtare clean.