SBML import: Incorrect processing of piecewise/Heaviside function

[dweindl]

Beer_MolBioSystems2014 has an assignment rule containing piecewise. This is correctly translated to a Heaviside function during import. However, there is no corresponding root function in the generated model.

Needs further investigation.

Reminds me of #2234.

[dweindl]

Similarly for Alkan_SciSignal2018, only that there the piecewise occurs inside a kineticLaw.

[dweindl]

So for Beer_MolBioSystems2014 the problem is:

AMICI/python/sdist/amici/de_model.py

Line 2275 in e89a1de

tmp_roots_old = self._collect_heaviside_roots(dxdt.args)

if dxdt is sympy.Heaviside, it won't be correctly processed, because only its arguments are checked.

More or less this line of code is present since more than 4 years (#1352). However, previously these models were imported correctly. I am not yet sure what exactly caused this problem. My guess is, that before, dxdt never was a plain Heaviside, but was always wrapped inside a sympy.Expr. I briefly tested sympy 1.12 and 1.11 and it still failed, so I think the relevant change was rather on the amici side than on the sympy side.

[FFroehlich]

What evidence do we have that models were imported correctly? Does the failed processing lead to an error?

[dweindl]

What evidence do we have that models were imported correctly?

Amici-0.11.28-generated model code from about 2 years ago that has:

void root_Beer_MolBioSystems2014(realtype *root, const realtype t, const realtype *x, const realtype *p, const realtype *k, const realtype *h, const realtype *tcl){
    root[0] = t - tau;
    root[1] = -t + tau;
}

EDIT: Found another correct one from AMICI v0.14.0, imported Dec 19 2022.

Does the failed processing lead to an error?

No, no error.

[dweindl]

Similarly for Alkan_SciSignal2018, only that there the piecewise occurs inside a kineticLaw.

False alarm for Alkan_SciSignal2018, there the Heaviside argument is a constant parameter. That's handled correctly.

[dweindl]

So for Beer_MolBioSystems2014 the problem is:

AMICI/python/sdist/amici/de_model.py

Line 2275 in e89a1de

tmp_roots_old = self._collect_heaviside_roots(dxdt.args)

if dxdt is sympy.Heaviside, it won't be correctly processed, because only its arguments are checked.

More or less this line of code is present since more than 4 years (#1352). However, previously these models were imported correctly.

This bug (or suicidal optimization), did not manifest before, because a per se meaningless multiplication with 1.0 in _parse_piecewise_to_heaviside ensured that dxdt would always be sympy.Expr. This changed very recently in b7a3e91.

This means, no AMICI-release was affected by this issue.