-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SBML import: conservation laws for non-constant species #1669
Conversation
Also: Modified the run-python-tests script to work also outside of Python virtual environments (venvs) by installing pip packages to the user's home directory.
Also, debugging null_space(...) call for stoichiometric matrix.
@@ -347,7 +347,8 @@ def sbml2amici(self, | |||
sbml.L3P_PARSE_LOG_AS_LN | |||
) | |||
self._process_sbml(constant_parameters) | |||
if self.symbols.get(SymbolId.EVENT, False): | |||
if self.symbols.get(SymbolId.EVENT, False) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the issue with heavisides in the flux vector, shouldn't break any conservation laws should it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. That "fix" was too specific for an SBML test case and misses the point.
The actual problem are conservation-law/x_rdata-dependent root functions:
AMICI/python/amici/ode_export.py
Lines 111 to 115 in 067826d
'root': | |
_FunctionInfo( | |
'realtype *root, const realtype t, const realtype *x, ' | |
'const realtype *p, const realtype *k, const realtype *h' | |
), |
Should be addressed accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me do that in a separate PR shortly.
python/amici/sbml_import.py
Outdated
assert len(state_idxs) == len(coefficients) | ||
|
||
# choose a state that is not already subject to removal | ||
# TODO is this necessary or can we just take the first one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with toposort + hierarchical expressions, I don't think this is necessary and we can just pick the first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Among the states involved in that CL, we still need to choose a state that has not been eliminated yet. Confused myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, true, that makes sense.
python/amici/conserved_moieties.py
Outdated
interactions of metabolites and reactions, and matrix of interaction | ||
""" | ||
dim = len(matched) | ||
MIN = 1e-9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my feeling is that MIN/MAX should be consistent across the different functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to agree.
T1 = initial_temperature | ||
e = math.exp(-1 / T1) | ||
while True: | ||
en = int(random.uniform(0, 1) * dim) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be great if the determination of conservation laws was deterministic. Use a fixed random seed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess that makes sense.
Co-authored-by: Fabian Fröhlich <fabian_froehlich@hms.harvard.edu>
SonarCloud Quality Gate failed. |
Extends identification conservation laws during SBML import to non-constant species based on the algorithm proposed in [1].
Parameterized stoichiometric coefficients are not supported. Neither are models with events/Heaviside/piecewise or RateRules. Conservation laws for non-constant species are completely ignored in this case.
Experimental feature: Enable by setting environment variable
AMICI_EXPERIMENTAL_SBML_NONCONST_CLS
to any value.[1] De Martino A, De Martino D, Mulet R, Pagnani A (2014) Identifying All Moiety Conservation Laws in Genome-Scale Metabolic Networks. PLoS ONE 9(7): e100750. https://doi.org/10.1371/journal.pone.0100750