Repository navigation
Conversation
…anl#1255) A compartment size read through a diamond of rules, r_i := sin(r_{i-1}) + cos(r_{i-2}), gave the dilution term a rate law whose functions write out exponentially in the depth and do not collect, and building the Simulator derived, checked and printed a derivative as long: 3.1 s at 12 rules, 57 s at 18. Past the inlining limit, a law that does not collect now declines the analytical Jacobian from its predicted length (kind written_out), in front of both the sympy and the native derivation paths, and the model runs on the finite-difference Jacobian: 0.02-0.03 s at any depth.
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 #1255.
What was wrong
Building a
Simulatorwith the analytical Jacobian (the default) took exponential time for a compartment whose size is read through a diamond of rules,r_i := sin(r_{i-1}) + cos(r_{i-2}).-Sn·(d cv/dt)/cv.r_{i-1} + r_{i-2}), but not this one, so it falls back to writing the text out.jacobian="fd"Past about 16 rules, main often ends on the finite-difference Jacobian anyway, after paying for it. Measured at 18 rules, beside a running suite: 28.7 s (25.6 s with
codegen=True), thendeclined: the build-time derivation exceeded its 20.0s budget.The GH #95 budget did not stop the cost, for two reasons:
sp.diff, and most of the time went after, walking and printing the derivative (19.5 M nodes traversed at 12 rules);_inline_functionscalls at 22 rules).The fix
A rate law past the inlining limit that does not collect now declines the analytical Jacobian before anything is written out.
_inlined_for_derivation, in front of both derivation paths (differentiate_rate_lawand the nativedifferentiate_rate_law_native)._INLINE_TEXT_LIMIT, 20,000 characters written out, past which Leftovers from clearing the variable-volume family (#787): 3 edge-case wrong answers, 4 slow paths, 3 over-refusals #1124 already tries_compact_inlined._inlined_length, which costs nothing.written_out, and names the law and its length.After: 0.02-0.03 s at any depth (12, 18, 22, 30 rules), and the run is bit-identical to
jacobian="fd".Why declining is the right answer here, not a loss: a derivative this long costs every Newton setup more to evaluate than differences of the right-hand side do. It is the same bet GH #95 makes, decided from the length instead of after the time is spent. On the three corpus models below that kept a complete analytical Jacobian, it bought neither run time nor accuracy:
Measured: the corpus, main against this branch
Real builds of 1,500 models (1,323 BioModels and 177
.net), 1,463 of which build on both sides. All runs usedcodegen=False, because the codegen cache key hashes bngsim's Python sources, so this branch compiles every model cold; the compile is not what this changes.Tests (
test_jacobian_written_out_decline.py, 9)written_out, and runs bit-identically tojacobian="fd"codegen=True_inline_functionspatched to raise), at 30 ruleswritten_outis a census kindThe existing
test_jacobian_diamond_of_rules.py(#1124) passes unchanged.Mutations
budgetChecked
pre-commit run --all-filesandsphinx -Ware clean.Not in this PR
_codegen.py's RHS paths,_switch_sensitivity). They are not in the default build path this issue measures.