From 9c603e1015ff0f233078f4e741b564bb9e9dc6c8 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 24 Oct 2024 10:53:08 +0200 Subject: [PATCH] PEtab import: Fix potentially incorrect sensitivities with state-dependent sigmas (#2562) Due to PEtab not allowing and observables in noiseFormula and AMICI not allowing state variables in sigma expressions, we are trying to replace any state variables by matching observables during PEtab import. However, if there were multiple observables with the same observableFormula, or one observableFormula was contained in another one, the substition of observableFormula for observableId could - depending on the ordering - result in noiseFormulas depending on other observables, which could lead to incorrect sensitivities due to https://github.com/AMICI-dev/AMICI/issues/2561. This change ensures that we are only using the observableId from the same row of the observable table as the noiseFormula. --------- Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> --- python/sdist/amici/petab/import_helpers.py | 26 ++++++++++------------ 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/python/sdist/amici/petab/import_helpers.py b/python/sdist/amici/petab/import_helpers.py index b52d74004d..70af87c3b3 100644 --- a/python/sdist/amici/petab/import_helpers.py +++ b/python/sdist/amici/petab/import_helpers.py @@ -47,8 +47,8 @@ def get_observation_model( observables = {} sigmas = {} - nan_pat = r"^[nN]a[nN]$" + for _, observable in observable_df.iterrows(): oid = str(observable.name) # need to sanitize due to https://github.com/PEtab-dev/PEtab/issues/447 @@ -56,20 +56,18 @@ def get_observation_model( formula_obs = re.sub(nan_pat, "", str(observable[OBSERVABLE_FORMULA])) formula_noise = re.sub(nan_pat, "", str(observable[NOISE_FORMULA])) observables[oid] = {"name": name, "formula": formula_obs} - sigmas[oid] = formula_noise - - # PEtab does currently not allow observables in noiseFormula and AMICI - # cannot handle states in sigma expressions. Therefore, where possible, - # replace species occurring in error model definition by observableIds. - replacements = { - sp.sympify(observable["formula"], locals=_clash): sp.Symbol( - observable_id + formula_noise_sym = sp.sympify(formula_noise, locals=_clash) + formula_obs_sym = sp.sympify(formula_obs, locals=_clash) + # PEtab does currently not allow observables in noiseFormula, and + # AMICI cannot handle state variables in sigma expressions. + # Therefore, where possible, replace state variables occurring in + # the error model definition by equivalent observableIds. + # Do not use observableIds from other rows + # (https://github.com/AMICI-dev/AMICI/issues/2561). + formula_noise_sym = formula_noise_sym.subs( + formula_obs_sym, sp.Symbol(oid) ) - for observable_id, observable in observables.items() - } - for observable_id, formula in sigmas.items(): - repl = sp.sympify(formula, locals=_clash).subs(replacements) - sigmas[observable_id] = str(repl) + sigmas[oid] = str(formula_noise_sym) noise_distrs = petab_noise_distributions_to_amici(observable_df)