account for heaviside functions in expressions#1382
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1382 +/- ##
============================================
+ Coverage 78.67% 90.88% +12.21%
============================================
Files 63 2 -61
Lines 9925 1404 -8521
============================================
- Hits 7808 1276 -6532
+ Misses 2117 128 -1989
Flags with carried forward coverage won't be shown. Click here to find out more.
|
paulstapor
left a comment
paulstapor
left a comment
There was a problem hiding this comment.
should hopefully do the job...
dilpath
left a comment
dilpath
left a comment
There was a problem hiding this comment.
Should have a test at some point I guess
will add Chen_MSB2009 which should now be working. |
python/amici/sbml_import.py
Outdated
| if isinstance(trigger, sp.core.relational.StrictLessThan): | ||
| # x < y => x - y < 0 => r < 0 | ||
| return sp.Heaviside(-root) | ||
| return 1 - sp.Heaviside(root) | ||
| if isinstance(trigger, sp.core.relational.LessThan): | ||
| # x <= y => not(y < x) => not(y - x < 0) => not -r < 0 | ||
| return 1 - sp.Heaviside(root) | ||
| return sp.Heaviside(-root) |
There was a problem hiding this comment.
What's the difference between 1-H(x) and H(-x)? Should the difference between < and <= be the value that H(x) is defined to take at x==0? Similarly for > and >= below?
There was a problem hiding this comment.
Ah, nevermind
AMICI/src/symbolic_functions.cpp
Lines 90 to 95 in a5c679b
There was a problem hiding this comment.
Hm, given this definition, x < 0 would correspond to H(-x)?
There was a problem hiding this comment.
Actually, 1-H(x) and H(-x) should be equivalent... For the difference between < and <= I have to say: Hard to tell what will happen if x is a floating point number...
There was a problem hiding this comment.
1-H(x)andH(-x)should be equivalent
Not equivalent for this implementation of the Heaviside function but agreed 👍 (equivalent everywhere except 0)
There was a problem hiding this comment.
This implementation addresses the situation when the simulation starts at a point where some of the arguments of heaviside functions evaluates to 0 (which can happen when translating SBML piecewise functions), which led to errors previously. It looks like the cpp implementation needs to be updated as its not consistent with the internal implementation. Will add a short blurb about that to the documentation.
Actually just disabling the model agains since that model is a mess (as reported before). I'm confident that implementation is correct, but llh values don't match. Also functionality is tested in Weber_BMC2015. |
|
SonarCloud Quality Gate failed. |
….13) Breaking changes: * AMICI requires Python>=3.7 * Updated package installation (PEP517/518): Creating source distributions requires https://github.com/pypa/build (#1384) (but now handles all package building dependencies properly) Features: * More flexible state reinitialization (#1417) Updated dependencies: * Upgrade to sundials 5.7.0 (#1392) Fixes: * Python: account for heaviside functions in expressions (#1382) * Python: allow loading of existing models in import_petab_problem (#1383) * Python: Don't override user-provided compiler/linker flags (#1389) * Python: PEtab import reinitialization fixes (#1417) * Python: Fix PEtab observables for pysb models (#1390) * Python: Substitute expressions in event condition expressions (#1404) * Python: Unspecified initial states in PEtab conditions table default to SBML initial value (#1397) * C++: Fix timepoint out of bounds access (#1402) * C++: Fix exported CMake config (#1388) * Fixed Dockerfile: add python3-venv (#1398, #1408) Other: * Slim exported swig interface (#1425) * Updated documentation * Getting started tutorial (#1423) * List supported SBML test tags (#1428) * Add AMICI C++/Python/Matlab feature comparison (#1409) * ... * Various minor CI improvements * ...
4 participants
No description provided.