From 80817eff51bc9b28ac6dfd2d1d4c6a70673b9c86 Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Wed, 7 Oct 2026 11:37:36 +0100 Subject: [PATCH 01/12] add option to convert model assignments to experiments in v1 to v2 converter --- petab/v2/petab1to2.py | 151 ++++++++++++++++++++++++- tests/v2/test_conversion.py | 220 ++++++++++++++++++++++++++++++++++++ 2 files changed, 370 insertions(+), 1 deletion(-) diff --git a/petab/v2/petab1to2.py b/petab/v2/petab1to2.py index 20263434..dfadf488 100644 --- a/petab/v2/petab1to2.py +++ b/petab/v2/petab1to2.py @@ -5,6 +5,7 @@ import math import shutil import warnings +from collections import defaultdict from contextlib import suppress from pathlib import Path from tempfile import TemporaryDirectory @@ -12,12 +13,16 @@ from uuid import uuid4 import pandas as pd +import sympy as sp from pandas.io.common import get_handle, is_url +from sbmlmath import TimeSymbol, sbml_math_to_sympy from .. import v1, v2 from ..v1.parameters import scale, unscale from ..v1.yaml import get_path_prefix, load_yaml from ..v1.yaml import validate as validate_yaml +from ..v1.math import petab_math_str, sympify_petab +from ..v1.yaml import get_path_prefix, load_yaml, validate from ..versions import get_major_version from .models import MODEL_TYPE_SBML @@ -29,6 +34,7 @@ def petab1to2( output_dir: Path | str | None = None, *, validate: bool = True, + assignments_to_experiments: bool = False, ) -> v2.Problem | None: """Convert from PEtab 1.0 to PEtab 2.0 format. @@ -56,6 +62,10 @@ def petab1to2( If ``True`` (default), both problems are validated. If ``False``, neither problem is validated. The ``yaml_config`` content is validated regardless. + :param assignments_to_experiments: + Whether to replace time-dependent piecewise assignments in the model, + such as ``k := dose * piecewise(0, time < t_dose, 1)``, by + experiment periods and conditions. :raises ValueError: If the input is invalid or, if ``validate`` is ``True``, if the input @@ -66,6 +76,12 @@ def petab1to2( with TemporaryDirectory() as tmp_dir: petab_files_1to2(yaml_config, tmp_dir, validate=validate) + return petab_files_1to2( + yaml_config, output_dir, assignments_to_experiments + ) + + with TemporaryDirectory() as tmp_dir: + petab_files_1to2(yaml_config, tmp_dir, assignments_to_experiments) return v2.Problem.from_yaml(Path(tmp_dir, Path(yaml_config).name)) @@ -74,6 +90,7 @@ def petab_files_1to2( output_dir: Path | str, *, validate: bool = True, + assignments_to_experiments: bool = False, ): """Convert PEtab files from PEtab 1.0 to PEtab 2.0. @@ -85,6 +102,8 @@ def petab_files_1to2( :param validate: Whether to lint the PEtab v1 problem before and the generated PEtab v2 problem after the conversion. See :func:`petab1to2`. + :param assignments_to_experiments: + See :func:`petab1to2`. :raises ValueError: If the input is invalid or, if ``validate`` is ``True``, if the input @@ -114,6 +133,11 @@ def petab_files_1to2( ) if validate and v1.lint_problem(petab_problem): raise ValueError("Provided PEtab problem does not pass linting.") + new_periods = ( + _assignments_to_periods(petab_problem) + if assignments_to_experiments + else {} + ) output_dir = Path(output_dir) @@ -133,7 +157,10 @@ def petab_files_1to2( for file in ( model.location for model in new_yaml_config.model_files.values() ): - _copy_file(get_src_path(file), Path(get_dest_path(file))) + if new_periods: + petab_problem.model.to_file(get_dest_path(file)) + else: + _copy_file(get_src_path(file), Path(get_dest_path(file))) # Update observable table for observable_file in new_yaml_config.observable_files: @@ -146,6 +173,8 @@ def petab_files_1to2( # Update condition table for condition_file in new_yaml_config.condition_files: condition_df = v1.get_condition_df(get_src_path(condition_file)) + if new_periods: + condition_df = petab_problem.condition_df condition_df = v1v2_condition_df(condition_df, petab_problem.model) v2.write_condition_df(condition_df, get_dest_path(condition_file)) @@ -169,6 +198,21 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str: if not preeq_cond_id and not v2_condition_id(sim_cond_id): # without pre-equilibration and without any changes, there is # nothing to be described by an experiment + # check whether the conditions will exist in the v2 condition table + sim_cond_exists = ( + petab_problem.condition_df.loc[sim_cond_id].notna().any() + ) + preeq_cond_exists = ( + preeq_cond_id + and petab_problem.condition_df.loc[preeq_cond_id].notna().any() + ) + if ( + not sim_cond_exists + and not preeq_cond_exists + and (sim_cond_id, preeq_cond_id) not in new_periods + ): + # if we have only all-NaN conditions, we don't create a new + # experiment return "" if preeq_cond_id: @@ -210,6 +254,16 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str: v2.C.CONDITION_ID: v2_condition_id(sim_cond_id), } ) + experiments.extend( + {v2.C.EXPERIMENT_ID: exp_id, v2.C.TIME: t, v2.C.CONDITION_ID: c} + for t, c in new_periods.get((sim_cond_id, preeq_cond_id), []) + ) + if new_periods: + # drop periods of conditions whose changes were all replaced + has_changes = petab_problem.condition_df.notna().any(axis=1) + experiments = [ + e for e in experiments if has_changes[e[v2.C.CONDITION_ID]] + ] if experiments: exp_table_path = output_dir / "experiments.tsv" if exp_table_path.exists(): @@ -349,6 +403,101 @@ def _copy_file(src: Path | str, dest: Path): shutil.copy(str(src), str(dest)) +def _assignments_to_periods( + problem: v1.Problem, +) -> dict[tuple[str, str], list[tuple[float, str]]]: + """Replace time-dependent piecewise assignments by experiment periods. + + Assignment rules and initial assignments containing + ``piecewise(before, time < t_switch, after)`` are removed from the model, + which keeps the value before the switch. The condition table columns they + depend on are replaced by new conditions that set the assignment target + at the start of the respective periods. + + :returns: The new ``(time, condition ID)`` periods for each measured + ``(simulation condition ID, preequilibration condition ID)`` pair. + """ + sbml_model = problem.model.sbml_model + condition_df = problem.condition_df + overrides = { + cid: row.dropna().to_dict() for cid, row in condition_df.iterrows() + } + pairs = list( + problem.get_simulation_conditions_from_measurement_df() + .reindex( + columns=[ + v1.C.SIMULATION_CONDITION_ID, + v1.C.PREEQUILIBRATION_CONDITION_ID, + ], + fill_value="", + ) + .itertuples(index=False) + ) + parameter_ids = set(problem.parameter_df.index) + periods = defaultdict(list) + # (target ID, target value) -> new condition ID + new_conditions = {} + replaced_columns = set() + + for assignment in [ + *(r for r in sbml_model.getListOfRules() if r.isAssignment()), + *sbml_model.getListOfInitialAssignments(), + ]: + expr = sbml_math_to_sympy(assignment) + pws = [pw for pw in expr.atoms(sp.Piecewise) if pw.has(TimeSymbol)] + if not pws: + continue + (pw,) = pws + (time,) = pw.atoms(TimeSymbol) + (before, switch), _ = pw.args + t_switch = sp.solve(switch.lhs - switch.rhs, time)[0] + + target_id = assignment.getId() + target = sbml_model.getParameter(target_id) + target.setValue(float(expr.subs(pw, before))) + target.setConstant(False) + assignment.removeFromParentAndDelete() + replaced_columns |= {str(s) for s in expr.free_symbols} + + for sim_id, preeq_id in pairs: + t_sim = float(t_switch.subs(overrides[sim_id])) + values = { + t: expr.subs(overrides[sim_id]).subs(time, t) + for t in (0.0, t_sim) + } + if preeq_id: + values[v2.C.TIME_PREEQUILIBRATION] = expr.subs( + overrides[preeq_id] + ).subs(time, 0) + elif {str(s) for s in values[t_sim].free_symbols} <= parameter_ids: + # the model already has the pre-switch value, and the switch + # is valid as first period (only parameter table symbols) + del values[0.0] + for t, value in values.items(): + cond_id = new_conditions.setdefault( + (target_id, value), f"cond_{len(new_conditions)}" + ) + periods[sim_id, preeq_id].append((t, cond_id)) + + problem.condition_df = pd.concat( + [ + condition_df.drop(columns=replaced_columns, errors="ignore"), + pd.DataFrame.from_dict( + { + cid: { + target_id: float(value) + if value.is_number + else petab_math_str(value) + } + for (target_id, value), cid in new_conditions.items() + }, + orient="index", + ), + ] + ).rename_axis(v1.C.CONDITION_ID) + return {pair: sorted(p) for pair, p in periods.items()} + + def v1v2_condition_df( condition_df: pd.DataFrame, model: v1.Model ) -> pd.DataFrame: diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 68685c37..9f11ae2a 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -6,11 +6,13 @@ import pytest from petab import v1, v2 +from petab.v1 import Problem as v1Problem from petab.v1.calculate import calculate_llh as calculate_llh_v1 from petab.v1.models.sbml_model import SbmlModel from petab.v1.priors import Prior from petab.v2 import Problem from petab.v2.calculate import calculate_llh as calculate_llh_v2 +from petab.v2.models.sbml_model import SbmlModel from petab.v2.petab1to2 import petab1to2, v1v2_observable_df @@ -611,3 +613,221 @@ def test_benchmark_collection(problem_id): pytest.skip(str(e)) assert isinstance(problem, Problem) assert len(problem.measurements) + + +def test_petab1to2_assignments_to_experiments(tmp_path): + problem = v1Problem() + ant_model = """ + model conversion + species A, B; + A = 10; + B = 0; + k1 = piecewise(1, time < 10, 2); + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + + problem.add_condition("c1") + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", "c1", time=0.0, measurement=10.0) + problem.add_measurement("obs_a", "c1", time=20.0, measurement=5.0) + problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) + + yaml_path = problem.to_files_generic(prefix_path=tmp_path) + + converted = petab1to2(yaml_path, assignments_to_experiments=True) + + # the value before the switch is kept in the model, so that only the + # switch itself needs a condition + assert converted.model.sbml_model.getParameter("k1").getValue() == 1.0 + + experiments_expected = pd.DataFrame( + data={ + v2.C.EXPERIMENT_ID: ["experiment____c1"], + v2.C.TIME: [10.0], + v2.C.CONDITION_ID: ["cond_0"], + } + ) + + conditions_expected = pd.DataFrame( + data={ + v2.C.CONDITION_ID: ["cond_0"], + v2.C.TARGET_ID: ["k1"], + v2.C.TARGET_VALUE: [2.0], + } + ) + + assert converted.experiment_df.equals(experiments_expected) + assert converted.condition_df.equals(conditions_expected) + assert converted.measurement_df[v2.C.EXPERIMENT_ID].tolist() == [ + "experiment____c1", + "experiment____c1", + ] + + +def test_petab1to2_assignments_to_experiments_nested(tmp_path): + """Test conversion of piecewise assignments that are nested inside a + larger expression and switch at a time given by some expression.""" + problem = v1Problem() + ant_model = """ + model conversion + species A, B; + A = 10; + B = 0; + level = 3; + k1 := level * piecewise(0, time - 10 < 0, 1); + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + + problem.add_condition("c1") + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", "c1", time=0.0, measurement=10.0) + problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) + + yaml_path = problem.to_files_generic(prefix_path=tmp_path) + + converted = petab1to2(yaml_path, assignments_to_experiments=True) + + assert converted.model.sbml_model.getParameter("k1").getValue() == 0.0 + + # `level` is not a parameter table entry, so it must not appear in the + # first period -- the pre-dose value is kept as a condition here + experiments_expected = pd.DataFrame( + data={ + v2.C.EXPERIMENT_ID: ["experiment____c1", "experiment____c1"], + v2.C.TIME: [0.0, 10.0], + v2.C.CONDITION_ID: ["cond_0", "cond_1"], + } + ) + + # the factor outside the piecewise expression is preserved + conditions_expected = pd.DataFrame( + data={ + v2.C.CONDITION_ID: ["cond_0", "cond_1"], + v2.C.TARGET_ID: ["k1", "k1"], + v2.C.TARGET_VALUE: [0.0, "level"], + } + ) + + assert converted.experiment_df.equals(experiments_expected) + assert converted.condition_df.equals(conditions_expected) + + +def test_petab1to2_assignments_to_experiments_condition_specific(tmp_path): + """Test conversion of piecewise assignments whose switching times and + values are set by the condition table.""" + problem = v1Problem() + ant_model = """ + model conversion + species A, B; + A = 10; + B = 0; + dose = 0; + dose_time = 0; + k1 := dose * piecewise(0, time - dose_time < 0, 1); + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + + problem.add_condition("c1", dose=2.0, dose_time=10.0) + problem.add_condition("c2", dose=3.0, dose_time=20.0) + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", "c1", time=30.0, measurement=10.0) + problem.add_measurement("obs_a", "c2", time=30.0, measurement=5.0) + problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) + + yaml_path = problem.to_files_generic(prefix_path=tmp_path) + + converted = petab1to2(yaml_path, assignments_to_experiments=True) + + # the pre-dose value is the same for both conditions and is kept in the + # model; each condition gets its own experiment and switching time + assert converted.model.sbml_model.getParameter("k1").getValue() == 0.0 + + experiments_expected = pd.DataFrame( + data={ + v2.C.EXPERIMENT_ID: ["experiment____c1", "experiment____c2"], + v2.C.TIME: [10.0, 20.0], + v2.C.CONDITION_ID: ["cond_0", "cond_1"], + } + ) + + # `dose` and `dose_time` are covered by the new conditions + conditions_expected = pd.DataFrame( + data={ + v2.C.CONDITION_ID: ["cond_0", "cond_1"], + v2.C.TARGET_ID: ["k1", "k1"], + v2.C.TARGET_VALUE: [2.0, 3.0], + } + ) + + assert converted.experiment_df.equals(experiments_expected) + assert converted.condition_df.equals(conditions_expected) + assert converted.measurement_df[v2.C.EXPERIMENT_ID].tolist() == [ + "experiment____c1", + "experiment____c2", + ] + + +def test_petab1to2_assignments_to_experiments_preequilibration(tmp_path): + """Test that the converted assignments are applied during + preequilibration, using the preequilibration condition's values.""" + problem = v1Problem() + ant_model = """ + model conversion + species A, B; + A = 10; + B = 0; + dose = 0; + dose_time = 0; + k1 := dose * piecewise(0, time - dose_time < 0, 1); + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + + problem.add_condition("c1", dose=2.0, dose_time=10.0) + # the dose is present from the start, i.e. also during preequilibration + problem.add_condition("c2", dose=3.0, dose_time=0.0) + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement( + "obs_a", "c1", time=30.0, measurement=10.0, preeq_cond_id="c2" + ) + problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) + + yaml_path = problem.to_files_generic(prefix_path=tmp_path) + + converted = petab1to2(yaml_path, assignments_to_experiments=True) + + # the dose is applied during preequilibration, so the reset to the + # pre-dose value at t=0 is not redundant here and must be kept + experiments_expected = pd.DataFrame( + data={ + v2.C.EXPERIMENT_ID: ["experiment__c2___c1"] * 3, + v2.C.TIME: [v2.C.TIME_PREEQUILIBRATION, 0.0, 10.0], + v2.C.CONDITION_ID: ["cond_2", "cond_0", "cond_1"], + } + ) + + conditions_expected = pd.DataFrame( + data={ + v2.C.CONDITION_ID: ["cond_0", "cond_1", "cond_2"], + v2.C.TARGET_ID: ["k1", "k1", "k1"], + v2.C.TARGET_VALUE: [0.0, 2.0, 3.0], + } + ) + + assert converted.experiment_df.equals(experiments_expected) + assert converted.condition_df.equals(conditions_expected) From 85f4b6b5949496571e87e1b0a81977e07b36d4cf Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Mon, 12 Oct 2026 10:49:38 +0100 Subject: [PATCH 02/12] make assignments to exps a separate function acting on v2 problem --- petab/v2/converters.py | 169 +++++++++++++++++++++++++++++++++++- petab/v2/petab1to2.py | 132 ++-------------------------- tests/v2/test_conversion.py | 80 ++++++++--------- 3 files changed, 213 insertions(+), 168 deletions(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index 33f6f550..8258be29 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -4,10 +4,11 @@ import warnings from copy import deepcopy +from itertools import count import libsbml import sympy as sp -from sbmlmath import sbml_math_to_sympy, set_math +from sbmlmath import TimeSymbol, sbml_math_to_sympy, set_math from .core import ( Change, @@ -20,7 +21,7 @@ from .models._sbml_utils import add_sbml_parameter, check from .models.sbml_model import SbmlModel -__all__ = ["ExperimentsToSbmlConverter"] +__all__ = ["ExperimentsToSbmlConverter", "assignments_to_experiments"] class ExperimentsToSbmlConverter: @@ -632,3 +633,167 @@ def _add_indicators_to_conditions(self) -> None: ] ) ] + + +DEFAULT_EXPERIMENT_ID = "default_experiment" + +def assignments_to_experiments(problem: Problem) -> Problem: + """Replace time-dependent piecewise assignments by experiment periods. + + Measurements without an experiment ID are assigned to a new experiment + :data:`DEFAULT_EXPERIMENT_ID`, if needed. + + :param problem: The PEtab problem to convert. This will not be modified. + :returns: The converted PEtab problem. + """ + if not isinstance(problem.model, SbmlModel): + raise TypeError("Only SBML models are supported.") + + problem = deepcopy(problem) + sbml_model = problem.model.sbml_model + parameter_ids = {p.id for p in problem.parameters} + + experiments = list(problem.experiments) + if any(m.experiment_id is None for m in problem.measurements): + default_experiment = Experiment(id=DEFAULT_EXPERIMENT_ID) + experiments.append(default_experiment) + else: + default_experiment = None + # periods that are dropped if all their conditions were replaced + droppable = { + id(p) for e in experiments for p in e.periods if p.condition_ids + } + for experiment in experiments: + if all(p.is_preequilibration for p in experiment.periods): + experiment += (period := ExperimentPeriod(time=0)) + droppable.add(id(period)) + + # (experiment, period start time, target ID, target value) + additions = [] + replaced_symbols = set() + + for assignment in [ + *(r for r in sbml_model.getListOfRules() if r.isAssignment()), + *sbml_model.getListOfInitialAssignments(), + ]: + expr = sbml_math_to_sympy(assignment) + pws = [pw for pw in expr.atoms(sp.Piecewise) if pw.has(TimeSymbol)] + if not pws: + continue + (pw,) = pws + (time,) = pw.atoms(TimeSymbol) + (before, switch), _ = pw.args + t_switch = sp.solve(switch.lhs - switch.rhs, time)[0] + + target_id = assignment.getId() + default = expr.subs(pw, before) + target = sbml_model.getParameter(target_id) + target.setValue(float(default)) + target.setConstant(False) + assignment.removeFromParentAndDelete() + symbol_ids = {str(s) for s in expr.free_symbols} + replaced_symbols |= symbol_ids + + for experiment in experiments: + periods = experiment.sorted_periods + overrides = {} + for i, period in enumerate(periods): + changes = problem.get_changes_for_period(period) + overrides |= {c.target_id: c.target_value for c in changes} + subs = { + s: overrides[str(s)] + for s in expr.free_symbols + if str(s) in overrides + } + cur_expr = expr.subs(subs) + if period.is_preequilibration: + additions.append( + ( + experiment, + period.time, + target_id, + cur_expr.subs(time, 0), + ) + ) + continue + + t_end = periods[i + 1].time if i + 1 < len(periods) else sp.oo + t = float(t_switch.subs(subs)) + switch_value = ( + cur_expr.subs(time, t) if period.time < t < t_end else None + ) + start_value = cur_expr.subs(time, period.time) + # The model already has the pre-switch value at the start of + # the first period, and the switch can become the first + # period if it only refers to parameter table symbols. + redundant = ( + i == 0 + and (start_value - default).is_zero + and ( + switch_value is None + or {str(s) for s in switch_value.free_symbols} + <= parameter_ids + ) + ) + if not redundant and ( + i == 0 or {c.target_id for c in changes} & symbol_ids + ): + additions.append( + (experiment, period.time, target_id, start_value) + ) + if switch_value is not None: + additions.append((experiment, t, target_id, switch_value)) + + # drop changes that are covered by the new conditions + condition_ids = {c.id for c in problem.conditions} + for table in problem.condition_tables: + for condition in table.conditions: + condition.changes = [ + c + for c in condition.changes + if c.target_id not in replaced_symbols + ] + table.elements = [c for c in table.conditions if c.changes] + removed_condition_ids = condition_ids - {c.id for c in problem.conditions} + for experiment in experiments: + for period in experiment.periods: + period.condition_ids = [ + cid + for cid in period.condition_ids + if cid not in removed_condition_ids + ] + + # add the new conditions to new or existing periods + new_conditions = {} + cond_counter = count() + for experiment, t, target_id, value in additions: + if (cond_id := new_conditions.get((target_id, value))) is None: + while (cond_id := f"cond_{next(cond_counter)}") in condition_ids: + pass + new_conditions[target_id, value] = cond_id + problem += Condition( + id=cond_id, + changes=[Change(target_id=target_id, target_value=value)], + ) + period = next((p for p in experiment.periods if p.time == t), None) + if period is None: + experiment += (period := ExperimentPeriod(time=t)) + period.condition_ids.append(cond_id) + + for experiment in experiments: + experiment.sort_periods() + experiment.periods = [ + p + for p in experiment.periods + if p.condition_ids or id(p) not in droppable + ] or experiment.periods[:1] + + if default_experiment and any( + p.condition_ids for p in default_experiment.periods + ): + problem += default_experiment + for measurement in problem.measurements: + if measurement.experiment_id is None: + measurement.experiment_id = DEFAULT_EXPERIMENT_ID + + return problem diff --git a/petab/v2/petab1to2.py b/petab/v2/petab1to2.py index dfadf488..6c782b27 100644 --- a/petab/v2/petab1to2.py +++ b/petab/v2/petab1to2.py @@ -5,7 +5,6 @@ import math import shutil import warnings -from collections import defaultdict from contextlib import suppress from pathlib import Path from tempfile import TemporaryDirectory @@ -13,16 +12,12 @@ from uuid import uuid4 import pandas as pd -import sympy as sp from pandas.io.common import get_handle, is_url -from sbmlmath import TimeSymbol, sbml_math_to_sympy from .. import v1, v2 from ..v1.parameters import scale, unscale from ..v1.yaml import get_path_prefix, load_yaml from ..v1.yaml import validate as validate_yaml -from ..v1.math import petab_math_str, sympify_petab -from ..v1.yaml import get_path_prefix, load_yaml, validate from ..versions import get_major_version from .models import MODEL_TYPE_SBML @@ -79,9 +74,10 @@ def petab1to2( return petab_files_1to2( yaml_config, output_dir, assignments_to_experiments ) + return petab_files_1to2(yaml_config, output_dir) with TemporaryDirectory() as tmp_dir: - petab_files_1to2(yaml_config, tmp_dir, assignments_to_experiments) + petab_files_1to2(yaml_config, tmp_dir) return v2.Problem.from_yaml(Path(tmp_dir, Path(yaml_config).name)) @@ -133,11 +129,6 @@ def petab_files_1to2( ) if validate and v1.lint_problem(petab_problem): raise ValueError("Provided PEtab problem does not pass linting.") - new_periods = ( - _assignments_to_periods(petab_problem) - if assignments_to_experiments - else {} - ) output_dir = Path(output_dir) @@ -157,10 +148,7 @@ def petab_files_1to2( for file in ( model.location for model in new_yaml_config.model_files.values() ): - if new_periods: - petab_problem.model.to_file(get_dest_path(file)) - else: - _copy_file(get_src_path(file), Path(get_dest_path(file))) + _copy_file(get_src_path(file), Path(get_dest_path(file))) # Update observable table for observable_file in new_yaml_config.observable_files: @@ -173,8 +161,6 @@ def petab_files_1to2( # Update condition table for condition_file in new_yaml_config.condition_files: condition_df = v1.get_condition_df(get_src_path(condition_file)) - if new_periods: - condition_df = petab_problem.condition_df condition_df = v1v2_condition_df(condition_df, petab_problem.model) v2.write_condition_df(condition_df, get_dest_path(condition_file)) @@ -198,6 +184,7 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str: if not preeq_cond_id and not v2_condition_id(sim_cond_id): # without pre-equilibration and without any changes, there is # nothing to be described by an experiment + return "" # check whether the conditions will exist in the v2 condition table sim_cond_exists = ( petab_problem.condition_df.loc[sim_cond_id].notna().any() @@ -206,11 +193,7 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str: preeq_cond_id and petab_problem.condition_df.loc[preeq_cond_id].notna().any() ) - if ( - not sim_cond_exists - and not preeq_cond_exists - and (sim_cond_id, preeq_cond_id) not in new_periods - ): + if not sim_cond_exists and not preeq_cond_exists: # if we have only all-NaN conditions, we don't create a new # experiment return "" @@ -254,16 +237,6 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str: v2.C.CONDITION_ID: v2_condition_id(sim_cond_id), } ) - experiments.extend( - {v2.C.EXPERIMENT_ID: exp_id, v2.C.TIME: t, v2.C.CONDITION_ID: c} - for t, c in new_periods.get((sim_cond_id, preeq_cond_id), []) - ) - if new_periods: - # drop periods of conditions whose changes were all replaced - has_changes = petab_problem.condition_df.notna().any(axis=1) - experiments = [ - e for e in experiments if has_changes[e[v2.C.CONDITION_ID]] - ] if experiments: exp_table_path = output_dir / "experiments.tsv" if exp_table_path.exists(): @@ -403,101 +376,6 @@ def _copy_file(src: Path | str, dest: Path): shutil.copy(str(src), str(dest)) -def _assignments_to_periods( - problem: v1.Problem, -) -> dict[tuple[str, str], list[tuple[float, str]]]: - """Replace time-dependent piecewise assignments by experiment periods. - - Assignment rules and initial assignments containing - ``piecewise(before, time < t_switch, after)`` are removed from the model, - which keeps the value before the switch. The condition table columns they - depend on are replaced by new conditions that set the assignment target - at the start of the respective periods. - - :returns: The new ``(time, condition ID)`` periods for each measured - ``(simulation condition ID, preequilibration condition ID)`` pair. - """ - sbml_model = problem.model.sbml_model - condition_df = problem.condition_df - overrides = { - cid: row.dropna().to_dict() for cid, row in condition_df.iterrows() - } - pairs = list( - problem.get_simulation_conditions_from_measurement_df() - .reindex( - columns=[ - v1.C.SIMULATION_CONDITION_ID, - v1.C.PREEQUILIBRATION_CONDITION_ID, - ], - fill_value="", - ) - .itertuples(index=False) - ) - parameter_ids = set(problem.parameter_df.index) - periods = defaultdict(list) - # (target ID, target value) -> new condition ID - new_conditions = {} - replaced_columns = set() - - for assignment in [ - *(r for r in sbml_model.getListOfRules() if r.isAssignment()), - *sbml_model.getListOfInitialAssignments(), - ]: - expr = sbml_math_to_sympy(assignment) - pws = [pw for pw in expr.atoms(sp.Piecewise) if pw.has(TimeSymbol)] - if not pws: - continue - (pw,) = pws - (time,) = pw.atoms(TimeSymbol) - (before, switch), _ = pw.args - t_switch = sp.solve(switch.lhs - switch.rhs, time)[0] - - target_id = assignment.getId() - target = sbml_model.getParameter(target_id) - target.setValue(float(expr.subs(pw, before))) - target.setConstant(False) - assignment.removeFromParentAndDelete() - replaced_columns |= {str(s) for s in expr.free_symbols} - - for sim_id, preeq_id in pairs: - t_sim = float(t_switch.subs(overrides[sim_id])) - values = { - t: expr.subs(overrides[sim_id]).subs(time, t) - for t in (0.0, t_sim) - } - if preeq_id: - values[v2.C.TIME_PREEQUILIBRATION] = expr.subs( - overrides[preeq_id] - ).subs(time, 0) - elif {str(s) for s in values[t_sim].free_symbols} <= parameter_ids: - # the model already has the pre-switch value, and the switch - # is valid as first period (only parameter table symbols) - del values[0.0] - for t, value in values.items(): - cond_id = new_conditions.setdefault( - (target_id, value), f"cond_{len(new_conditions)}" - ) - periods[sim_id, preeq_id].append((t, cond_id)) - - problem.condition_df = pd.concat( - [ - condition_df.drop(columns=replaced_columns, errors="ignore"), - pd.DataFrame.from_dict( - { - cid: { - target_id: float(value) - if value.is_number - else petab_math_str(value) - } - for (target_id, value), cid in new_conditions.items() - }, - orient="index", - ), - ] - ).rename_axis(v1.C.CONDITION_ID) - return {pair: sorted(p) for pair, p in periods.items()} - - def v1v2_condition_df( condition_df: pd.DataFrame, model: v1.Model ) -> pd.DataFrame: diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 9f11ae2a..0af16133 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -6,12 +6,12 @@ import pytest from petab import v1, v2 -from petab.v1 import Problem as v1Problem from petab.v1.calculate import calculate_llh as calculate_llh_v1 from petab.v1.models.sbml_model import SbmlModel from petab.v1.priors import Prior from petab.v2 import Problem from petab.v2.calculate import calculate_llh as calculate_llh_v2 +from petab.v2.converters import assignments_to_experiments from petab.v2.models.sbml_model import SbmlModel from petab.v2.petab1to2 import petab1to2, v1v2_observable_df @@ -615,8 +615,8 @@ def test_benchmark_collection(problem_id): assert len(problem.measurements) -def test_petab1to2_assignments_to_experiments(tmp_path): - problem = v1Problem() +def test_assignments_to_experiments(): + problem = Problem() ant_model = """ model conversion species A, B; @@ -630,15 +630,12 @@ def test_petab1to2_assignments_to_experiments(tmp_path): """ problem.model = SbmlModel.from_antimony(ant_model) - problem.add_condition("c1") problem.add_observable("obs_a", formula="A", noise_formula=1.0) - problem.add_measurement("obs_a", "c1", time=0.0, measurement=10.0) - problem.add_measurement("obs_a", "c1", time=20.0, measurement=5.0) - problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) + problem.add_measurement("obs_a", time=0.0, measurement=10.0) + problem.add_measurement("obs_a", time=20.0, measurement=5.0) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - yaml_path = problem.to_files_generic(prefix_path=tmp_path) - - converted = petab1to2(yaml_path, assignments_to_experiments=True) + converted = assignments_to_experiments(problem) # the value before the switch is kept in the model, so that only the # switch itself needs a condition @@ -646,7 +643,7 @@ def test_petab1to2_assignments_to_experiments(tmp_path): experiments_expected = pd.DataFrame( data={ - v2.C.EXPERIMENT_ID: ["experiment____c1"], + v2.C.EXPERIMENT_ID: ["default_experiment"], v2.C.TIME: [10.0], v2.C.CONDITION_ID: ["cond_0"], } @@ -663,15 +660,15 @@ def test_petab1to2_assignments_to_experiments(tmp_path): assert converted.experiment_df.equals(experiments_expected) assert converted.condition_df.equals(conditions_expected) assert converted.measurement_df[v2.C.EXPERIMENT_ID].tolist() == [ - "experiment____c1", - "experiment____c1", + "default_experiment", + "default_experiment", ] -def test_petab1to2_assignments_to_experiments_nested(tmp_path): +def test_assignments_to_experiments_nested(): """Test conversion of piecewise assignments that are nested inside a larger expression and switch at a time given by some expression.""" - problem = v1Problem() + problem = Problem() ant_model = """ model conversion species A, B; @@ -686,14 +683,11 @@ def test_petab1to2_assignments_to_experiments_nested(tmp_path): """ problem.model = SbmlModel.from_antimony(ant_model) - problem.add_condition("c1") problem.add_observable("obs_a", formula="A", noise_formula=1.0) - problem.add_measurement("obs_a", "c1", time=0.0, measurement=10.0) - problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) + problem.add_measurement("obs_a", time=0.0, measurement=10.0) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - yaml_path = problem.to_files_generic(prefix_path=tmp_path) - - converted = petab1to2(yaml_path, assignments_to_experiments=True) + converted = assignments_to_experiments(problem) assert converted.model.sbml_model.getParameter("k1").getValue() == 0.0 @@ -701,7 +695,7 @@ def test_petab1to2_assignments_to_experiments_nested(tmp_path): # first period -- the pre-dose value is kept as a condition here experiments_expected = pd.DataFrame( data={ - v2.C.EXPERIMENT_ID: ["experiment____c1", "experiment____c1"], + v2.C.EXPERIMENT_ID: ["default_experiment"] * 2, v2.C.TIME: [0.0, 10.0], v2.C.CONDITION_ID: ["cond_0", "cond_1"], } @@ -720,10 +714,10 @@ def test_petab1to2_assignments_to_experiments_nested(tmp_path): assert converted.condition_df.equals(conditions_expected) -def test_petab1to2_assignments_to_experiments_condition_specific(tmp_path): +def test_assignments_to_experiments_condition_specific(): """Test conversion of piecewise assignments whose switching times and values are set by the condition table.""" - problem = v1Problem() + problem = Problem() ant_model = """ model conversion species A, B; @@ -741,14 +735,18 @@ def test_petab1to2_assignments_to_experiments_condition_specific(tmp_path): problem.add_condition("c1", dose=2.0, dose_time=10.0) problem.add_condition("c2", dose=3.0, dose_time=20.0) + problem.add_experiment("experiment____c1", 0, "c1") + problem.add_experiment("experiment____c2", 0, "c2") problem.add_observable("obs_a", formula="A", noise_formula=1.0) - problem.add_measurement("obs_a", "c1", time=30.0, measurement=10.0) - problem.add_measurement("obs_a", "c2", time=30.0, measurement=5.0) - problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) - - yaml_path = problem.to_files_generic(prefix_path=tmp_path) + problem.add_measurement( + "obs_a", experiment_id="experiment____c1", time=30.0, measurement=10.0 + ) + problem.add_measurement( + "obs_a", experiment_id="experiment____c2", time=30.0, measurement=5.0 + ) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - converted = petab1to2(yaml_path, assignments_to_experiments=True) + converted = assignments_to_experiments(problem) # the pre-dose value is the same for both conditions and is kept in the # model; each condition gets its own experiment and switching time @@ -779,10 +777,10 @@ def test_petab1to2_assignments_to_experiments_condition_specific(tmp_path): ] -def test_petab1to2_assignments_to_experiments_preequilibration(tmp_path): +def test_assignments_to_experiments_preequilibration(): """Test that the converted assignments are applied during preequilibration, using the preequilibration condition's values.""" - problem = v1Problem() + problem = Problem() ant_model = """ model conversion species A, B; @@ -801,15 +799,19 @@ def test_petab1to2_assignments_to_experiments_preequilibration(tmp_path): problem.add_condition("c1", dose=2.0, dose_time=10.0) # the dose is present from the start, i.e. also during preequilibration problem.add_condition("c2", dose=3.0, dose_time=0.0) + problem.add_experiment( + "experiment__c2___c1", v2.C.TIME_PREEQUILIBRATION, "c2", 0, "c1" + ) problem.add_observable("obs_a", formula="A", noise_formula=1.0) problem.add_measurement( - "obs_a", "c1", time=30.0, measurement=10.0, preeq_cond_id="c2" + "obs_a", + experiment_id="experiment__c2___c1", + time=30.0, + measurement=10.0, ) - problem.add_parameter("k2", estimate=True, scale="lin", lb=1e-3, ub=1e3) - - yaml_path = problem.to_files_generic(prefix_path=tmp_path) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - converted = petab1to2(yaml_path, assignments_to_experiments=True) + converted = assignments_to_experiments(problem) # the dose is applied during preequilibration, so the reset to the # pre-dose value at t=0 is not redundant here and must be kept @@ -817,7 +819,7 @@ def test_petab1to2_assignments_to_experiments_preequilibration(tmp_path): data={ v2.C.EXPERIMENT_ID: ["experiment__c2___c1"] * 3, v2.C.TIME: [v2.C.TIME_PREEQUILIBRATION, 0.0, 10.0], - v2.C.CONDITION_ID: ["cond_2", "cond_0", "cond_1"], + v2.C.CONDITION_ID: ["cond_0", "cond_1", "cond_2"], } ) @@ -825,7 +827,7 @@ def test_petab1to2_assignments_to_experiments_preequilibration(tmp_path): data={ v2.C.CONDITION_ID: ["cond_0", "cond_1", "cond_2"], v2.C.TARGET_ID: ["k1", "k1", "k1"], - v2.C.TARGET_VALUE: [0.0, 2.0, 3.0], + v2.C.TARGET_VALUE: [3.0, 0.0, 2.0], } ) From 2adf87ad86f2e3eef87315d38a0b297d219b765b Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Mon, 12 Oct 2026 11:00:36 +0100 Subject: [PATCH 03/12] throw not implemented for multi-piecewise cases --- petab/v2/converters.py | 7 ++++ tests/v2/test_conversion.py | 81 +++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index 8258be29..32cab251 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -637,6 +637,7 @@ def _add_indicators_to_conditions(self) -> None: DEFAULT_EXPERIMENT_ID = "default_experiment" + def assignments_to_experiments(problem: Problem) -> Problem: """Replace time-dependent piecewise assignments by experiment periods. @@ -680,6 +681,12 @@ def assignments_to_experiments(problem: Problem) -> Problem: pws = [pw for pw in expr.atoms(sp.Piecewise) if pw.has(TimeSymbol)] if not pws: continue + if len(pws) > 1 or len(pws[0].args) != 2: + raise NotImplementedError( + "Only assignments with a single time-dependent piecewise " + "function with a single condition are supported, but " + f"the assignment to `{assignment.getId()}` is `{expr}`." + ) (pw,) = pws (time,) = pw.atoms(TimeSymbol) (before, switch), _ = pw.args diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 0af16133..302abc01 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -665,6 +665,56 @@ def test_assignments_to_experiments(): ] +def test_assignments_to_experiments_reversed(): + problem = Problem() + ant_model = """ + model conversion + species A, B; + A = 10; + B = 0; + k1 = piecewise(1, 10 > time, 2); + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", time=0.0, measurement=10.0) + problem.add_measurement("obs_a", time=20.0, measurement=5.0) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) + + converted = assignments_to_experiments(problem) + + # the value before the switch is kept in the model, so that only the + # switch itself needs a condition + assert converted.model.sbml_model.getParameter("k1").getValue() == 1.0 + + experiments_expected = pd.DataFrame( + data={ + v2.C.EXPERIMENT_ID: ["default_experiment"], + v2.C.TIME: [10.0], + v2.C.CONDITION_ID: ["cond_0"], + } + ) + + conditions_expected = pd.DataFrame( + data={ + v2.C.CONDITION_ID: ["cond_0"], + v2.C.TARGET_ID: ["k1"], + v2.C.TARGET_VALUE: [2.0], + } + ) + + assert converted.experiment_df.equals(experiments_expected) + assert converted.condition_df.equals(conditions_expected) + assert converted.measurement_df[v2.C.EXPERIMENT_ID].tolist() == [ + "default_experiment", + "default_experiment", + ] + + def test_assignments_to_experiments_nested(): """Test conversion of piecewise assignments that are nested inside a larger expression and switch at a time given by some expression.""" @@ -833,3 +883,34 @@ def test_assignments_to_experiments_preequilibration(): assert converted.experiment_df.equals(experiments_expected) assert converted.condition_df.equals(conditions_expected) + + +@pytest.mark.parametrize( + "assignment", + [ + "piecewise(0, time < 10, 1) + piecewise(0, time < 20, 1)", + "piecewise(0, time < 10, 1, time < 20, 2)", + ], +) +def test_assignments_to_experiments_multiple_switches(assignment): + """Test that assignments with more than one switch are rejected.""" + problem = Problem() + ant_model = f""" + model conversion + species A, B; + A = 10; + B = 0; + k1 := {assignment}; + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", time=0.0, measurement=10.0) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) + + with pytest.raises(NotImplementedError, match="single time-dependent"): + assignments_to_experiments(problem) From 09ec29141adc03ff47adbf41d2b0e11e1a5c9912 Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Mon, 12 Oct 2026 11:13:09 +0100 Subject: [PATCH 04/12] guard around t switch unresolveable --- petab/v2/converters.py | 10 +++++++++- tests/v2/test_conversion.py | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index 32cab251..99b2ed7a 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -725,7 +725,15 @@ def assignments_to_experiments(problem: Problem) -> Problem: continue t_end = periods[i + 1].time if i + 1 < len(periods) else sp.oo - t = float(t_switch.subs(subs)) + t = t_switch.subs(subs) + if not (t.is_number and t.is_real): + raise NotImplementedError( + "Only switching times that evaluate to a real number " + "are supported, but the switching time of the " + f"assignment to `{target_id}` in experiment " + f"`{experiment.id}` is `{t}`." + ) + t = float(t) switch_value = ( cur_expr.subs(time, t) if period.time < t < t_end else None ) diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 302abc01..db3583b1 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -914,3 +914,42 @@ def test_assignments_to_experiments_multiple_switches(assignment): with pytest.raises(NotImplementedError, match="single time-dependent"): assignments_to_experiments(problem) + + +@pytest.mark.parametrize( + "assignment, estimated", + [ + # depends on a species + ("piecewise(0, time < A, 1)", None), + ("piecewise(0, A * time < 5, 1)", None), + # depends on an estimated parameter + ("piecewise(0, time < t_dose, 1)", "t_dose"), + # no real solution + ("piecewise(0, time^2 + 1 < 0, 1)", None), + ], +) +def test_assignments_to_experiments_non_numeric_switch(assignment, estimated): + """Test that switching times that are not real numbers are rejected.""" + problem = Problem() + ant_model = f""" + model conversion + species A, B; + A = 10; + B = 0; + t_dose = 5; + k1 := {assignment}; + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", time=0.0, measurement=10.0) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) + if estimated: + problem.add_parameter(estimated, estimate=True, lb=0, ub=10) + + with pytest.raises(NotImplementedError, match="evaluate to a real number"): + assignments_to_experiments(problem) From cb4d06baf02346b00250b7df8ead09688f014044 Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Mon, 12 Oct 2026 11:44:37 +0100 Subject: [PATCH 05/12] fix up rebase --- petab/v2/petab1to2.py | 27 --------------------------- tests/v2/test_conversion.py | 1 - 2 files changed, 28 deletions(-) diff --git a/petab/v2/petab1to2.py b/petab/v2/petab1to2.py index 6c782b27..20263434 100644 --- a/petab/v2/petab1to2.py +++ b/petab/v2/petab1to2.py @@ -29,7 +29,6 @@ def petab1to2( output_dir: Path | str | None = None, *, validate: bool = True, - assignments_to_experiments: bool = False, ) -> v2.Problem | None: """Convert from PEtab 1.0 to PEtab 2.0 format. @@ -57,10 +56,6 @@ def petab1to2( If ``True`` (default), both problems are validated. If ``False``, neither problem is validated. The ``yaml_config`` content is validated regardless. - :param assignments_to_experiments: - Whether to replace time-dependent piecewise assignments in the model, - such as ``k := dose * piecewise(0, time < t_dose, 1)``, by - experiment periods and conditions. :raises ValueError: If the input is invalid or, if ``validate`` is ``True``, if the input @@ -71,13 +66,6 @@ def petab1to2( with TemporaryDirectory() as tmp_dir: petab_files_1to2(yaml_config, tmp_dir, validate=validate) - return petab_files_1to2( - yaml_config, output_dir, assignments_to_experiments - ) - return petab_files_1to2(yaml_config, output_dir) - - with TemporaryDirectory() as tmp_dir: - petab_files_1to2(yaml_config, tmp_dir) return v2.Problem.from_yaml(Path(tmp_dir, Path(yaml_config).name)) @@ -86,7 +74,6 @@ def petab_files_1to2( output_dir: Path | str, *, validate: bool = True, - assignments_to_experiments: bool = False, ): """Convert PEtab files from PEtab 1.0 to PEtab 2.0. @@ -98,8 +85,6 @@ def petab_files_1to2( :param validate: Whether to lint the PEtab v1 problem before and the generated PEtab v2 problem after the conversion. See :func:`petab1to2`. - :param assignments_to_experiments: - See :func:`petab1to2`. :raises ValueError: If the input is invalid or, if ``validate`` is ``True``, if the input @@ -185,18 +170,6 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str: # without pre-equilibration and without any changes, there is # nothing to be described by an experiment return "" - # check whether the conditions will exist in the v2 condition table - sim_cond_exists = ( - petab_problem.condition_df.loc[sim_cond_id].notna().any() - ) - preeq_cond_exists = ( - preeq_cond_id - and petab_problem.condition_df.loc[preeq_cond_id].notna().any() - ) - if not sim_cond_exists and not preeq_cond_exists: - # if we have only all-NaN conditions, we don't create a new - # experiment - return "" if preeq_cond_id: preeq_cond_id = f"{preeq_cond_id}_" diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index db3583b1..762c6667 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -12,7 +12,6 @@ from petab.v2 import Problem from petab.v2.calculate import calculate_llh as calculate_llh_v2 from petab.v2.converters import assignments_to_experiments -from petab.v2.models.sbml_model import SbmlModel from petab.v2.petab1to2 import petab1to2, v1v2_observable_df From 2f2c2aa17c9add82b5a48a96fe1eff2cecde44e7 Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Tue, 13 Oct 2026 15:23:24 +0100 Subject: [PATCH 06/12] fix to support both switch directions --- petab/v2/converters.py | 58 ++++++++++----- tests/v2/test_conversion.py | 142 +++++++++++++++++++++++++++++++++--- 2 files changed, 173 insertions(+), 27 deletions(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index 99b2ed7a..2d6bd6f5 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -689,11 +689,27 @@ def assignments_to_experiments(problem: Problem) -> Problem: ) (pw,) = pws (time,) = pw.atoms(TimeSymbol) - (before, switch), _ = pw.args - t_switch = sp.solve(switch.lhs - switch.rhs, time)[0] - + (value, switch), (otherwise, _) = pw.args target_id = assignment.getId() + + # `switch` is equivalent to `g > 0` (or `g >= 0`), so it holds after + # the switch if `g` increases with time, and before it otherwise + g = switch.gts - switch.lts + slope = sp.diff(g, time) + if slope.is_positive: + before, after = otherwise, value + elif slope.is_negative: + before, after = value, otherwise + else: + raise NotImplementedError( + "Cannot determine whether the condition " + f"`{switch}` of the assignment to `{target_id}` holds " + "before or after the switch." + ) + t_switch = sp.solve(g, time)[0] + default = expr.subs(pw, before) + after_expr = expr.subs(pw, after) target = sbml_model.getParameter(target_id) target.setValue(float(default)) target.setConstant(False) @@ -712,19 +728,6 @@ def assignments_to_experiments(problem: Problem) -> Problem: for s in expr.free_symbols if str(s) in overrides } - cur_expr = expr.subs(subs) - if period.is_preequilibration: - additions.append( - ( - experiment, - period.time, - target_id, - cur_expr.subs(time, 0), - ) - ) - continue - - t_end = periods[i + 1].time if i + 1 < len(periods) else sp.oo t = t_switch.subs(subs) if not (t.is_number and t.is_real): raise NotImplementedError( @@ -734,10 +737,29 @@ def assignments_to_experiments(problem: Problem) -> Problem: f"`{experiment.id}` is `{t}`." ) t = float(t) + cur_before, cur_after = ( + default.subs(subs), + after_expr.subs(subs), + ) + + if period.is_preequilibration: + # the value at time 0, which respects whether the + # condition holds at a switch at time 0 + value = expr.subs(subs).subs(time, 0) + additions.append( + (experiment, period.time, target_id, value) + ) + continue + + t_end = periods[i + 1].time if i + 1 < len(periods) else sp.oo switch_value = ( - cur_expr.subs(time, t) if period.time < t < t_end else None + cur_after.subs(time, t) + if period.time < t < t_end + else None ) - start_value = cur_expr.subs(time, period.time) + start_value = ( + cur_before if period.time < t else cur_after + ).subs(time, period.time) # The model already has the pre-switch value at the start of # the first period, and the switch can become the first # period if it only refers to parameter table symbols. diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 762c6667..61ed6646 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -916,19 +916,31 @@ def test_assignments_to_experiments_multiple_switches(assignment): @pytest.mark.parametrize( - "assignment, estimated", + "assignment, estimated, match", [ # depends on a species - ("piecewise(0, time < A, 1)", None), - ("piecewise(0, A * time < 5, 1)", None), + ("piecewise(0, time < A, 1)", None, "evaluate to a real number"), + # switch direction depends on the sign of a species + ("piecewise(0, A * time < 5, 1)", None, "before or after the switch"), # depends on an estimated parameter - ("piecewise(0, time < t_dose, 1)", "t_dose"), - # no real solution - ("piecewise(0, time^2 + 1 < 0, 1)", None), + ( + "piecewise(0, time < t_dose, 1)", + "t_dose", + "evaluate to a real number", + ), + # no real solution, and not monotonic in time + ( + "piecewise(0, time^2 + 1 < 0, 1)", + None, + "before or after the switch", + ), ], ) -def test_assignments_to_experiments_non_numeric_switch(assignment, estimated): - """Test that switching times that are not real numbers are rejected.""" +def test_assignments_to_experiments_non_numeric_switch( + assignment, estimated, match +): + """Test that switching times that are not real numbers, or switches + in an unknown direction, are rejected.""" problem = Problem() ant_model = f""" model conversion @@ -950,5 +962,117 @@ def test_assignments_to_experiments_non_numeric_switch(assignment, estimated): if estimated: problem.add_parameter(estimated, estimate=True, lb=0, ub=10) - with pytest.raises(NotImplementedError, match="evaluate to a real number"): + with pytest.raises(NotImplementedError, match=match): assignments_to_experiments(problem) + + +def test_assignments_to_experiments_switch_direction(): + """Test that `time < t` and `t < time` switch in opposite directions.""" + + def convert(assignment): + problem = Problem() + ant_model = f""" + model conversion + species A, B; + A = 10; + B = 0; + k1 := {assignment}; + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", time=20.0, measurement=5.0) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) + return assignments_to_experiments(problem) + + # k1 is 0 before and 1 after t=10 + converted_lt = convert("piecewise(0, time < 10, 1)") + # k1 is 1 before and 0 after t=10 + converted_gt = convert("piecewise(0, 10 < time, 1)") + + # the value before the switch is kept in the model + assert converted_lt.model.sbml_model.getParameter("k1").getValue() == 0.0 + assert converted_gt.model.sbml_model.getParameter("k1").getValue() == 1.0 + + experiments_expected = pd.DataFrame( + data={ + v2.C.EXPERIMENT_ID: ["default_experiment"], + v2.C.TIME: [10.0], + v2.C.CONDITION_ID: ["cond_0"], + } + ) + assert converted_lt.experiment_df.equals(experiments_expected) + assert converted_gt.experiment_df.equals(experiments_expected) + + # the switch sets k1 to the value after t=10 + assert converted_lt.condition_df.equals( + pd.DataFrame( + data={ + v2.C.CONDITION_ID: ["cond_0"], + v2.C.TARGET_ID: ["k1"], + v2.C.TARGET_VALUE: [1.0], + } + ) + ) + assert converted_gt.condition_df.equals( + pd.DataFrame( + data={ + v2.C.CONDITION_ID: ["cond_0"], + v2.C.TARGET_ID: ["k1"], + v2.C.TARGET_VALUE: [0.0], + } + ) + ) + + +@pytest.mark.parametrize("t_switch", [0, 10]) +@pytest.mark.parametrize( + "condition, holds_at_switch, holds_before", + [ + ("time < {t}", False, True), + ("time <= {t}", True, True), + ("time > {t}", False, False), + ("time >= {t}", True, False), + ("{t} < time", False, False), + ("{t} >= time", True, True), + ], +) +def test_assignments_to_experiments_preequilibration_boundary( + t_switch, condition, holds_at_switch, holds_before +): + """Test that preequilibration uses the assignment's value at time 0, + including whether the condition holds for a switch at time 0.""" + condition = condition.format(t=t_switch) + problem = Problem() + ant_model = f""" + model conversion + species A, B; + A = 10; + B = 0; + k1 := piecewise(0, {condition}, 1); + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + problem.add_experiment("e1", v2.C.TIME_PREEQUILIBRATION, [], 0, []) + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement( + "obs_a", experiment_id="e1", time=20.0, measurement=5.0 + ) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) + + converted = assignments_to_experiments(problem) + + holds_at_0 = holds_at_switch if t_switch == 0 else holds_before + (preeq_period,) = ( + p for p in converted.experiments[0].periods if p.is_preequilibration + ) + assert [ + (c.target_id, float(c.target_value)) + for c in converted.get_changes_for_period(preeq_period) + ] == [("k1", 0.0 if holds_at_0 else 1.0)] From 047c688e5d889803322623afdb4c5a528797ff8c Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Tue, 13 Oct 2026 16:03:11 +0100 Subject: [PATCH 07/12] review feedback on preeq and linting --- petab/v2/converters.py | 29 ++++++++++++++++++++++++++++- tests/v2/test_conversion.py | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index 2d6bd6f5..ebc74f0b 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -644,8 +644,16 @@ def assignments_to_experiments(problem: Problem) -> Problem: Measurements without an experiment ID are assigned to a new experiment :data:`DEFAULT_EXPERIMENT_ID`, if needed. + .. warning:: + + Switches during preequilibration are not supported. The problem + generated by this conversion has the assignment target keep its value + at time 0 throughout preequilibration. + :param problem: The PEtab problem to convert. This will not be modified. :returns: The converted PEtab problem. + :raises ValueError: + If the converted problem does not pass linting. """ if not isinstance(problem.model, SbmlModel): raise TypeError("Only SBML models are supported.") @@ -672,6 +680,7 @@ def assignments_to_experiments(problem: Problem) -> Problem: # (experiment, period start time, target ID, target value) additions = [] replaced_symbols = set() + converted_ids = [] for assignment in [ *(r for r in sbml_model.getListOfRules() if r.isAssignment()), @@ -716,6 +725,7 @@ def assignments_to_experiments(problem: Problem) -> Problem: assignment.removeFromParentAndDelete() symbol_ids = {str(s) for s in expr.free_symbols} replaced_symbols |= symbol_ids + converted_ids.append(target_id) for experiment in experiments: periods = experiment.sorted_periods @@ -729,7 +739,7 @@ def assignments_to_experiments(problem: Problem) -> Problem: if str(s) in overrides } t = t_switch.subs(subs) - if not (t.is_number and t.is_real): + if not (t.is_number and t.is_real and t.is_finite): raise NotImplementedError( "Only switching times that evaluate to a real number " "are supported, but the switching time of the " @@ -781,6 +791,16 @@ def assignments_to_experiments(problem: Problem) -> Problem: if switch_value is not None: additions.append((experiment, t, target_id, switch_value)) + if converted_ids and any(e.has_preequilibration for e in experiments): + warnings.warn( + "Switches of time-dependent piecewise assignments during " + "preequilibration are not supported. The converted " + f"assignments to {converted_ids} keep their value at time 0 " + "throughout " + "preequilibration.", + stacklevel=2, + ) + # drop changes that are covered by the new conditions condition_ids = {c.id for c in problem.conditions} for table in problem.condition_tables: @@ -833,4 +853,11 @@ def assignments_to_experiments(problem: Problem) -> Problem: if measurement.experiment_id is None: measurement.experiment_id = DEFAULT_EXPERIMENT_ID + validation_issues = problem.validate() + if validation_issues.has_errors(): + messages = "\n".join(f"{issue.message}" for issue in validation_issues) + raise ValueError( + f"The converted PEtab problem did not pass linting:\n{messages}" + ) + return problem diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 61ed6646..ef66d344 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -860,7 +860,8 @@ def test_assignments_to_experiments_preequilibration(): ) problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - converted = assignments_to_experiments(problem) + with pytest.warns(UserWarning, match="during preequilibration"): + converted = assignments_to_experiments(problem) # the dose is applied during preequilibration, so the reset to the # pre-dose value at t=0 is not redundant here and must be kept @@ -1066,7 +1067,8 @@ def test_assignments_to_experiments_preequilibration_boundary( ) problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - converted = assignments_to_experiments(problem) + with pytest.warns(UserWarning, match="during preequilibration"): + converted = assignments_to_experiments(problem) holds_at_0 = holds_at_switch if t_switch == 0 else holds_before (preeq_period,) = ( @@ -1076,3 +1078,32 @@ def test_assignments_to_experiments_preequilibration_boundary( (c.target_id, float(c.target_value)) for c in converted.get_changes_for_period(preeq_period) ] == [("k1", 0.0 if holds_at_0 else 1.0)] + + +def test_assignments_to_experiments_validate(): + """Test that the converted problem is linted. + + Here, the input problem is valid, but the conversion makes the estimated + parameter `k1` a condition table target. + """ + problem = Problem() + ant_model = """ + model conversion + species A, B; + A = 10; + B = 0; + k1 = piecewise(0, time < 10, 1); + k2 = 0.5; + R1: A -> B; k1 * A; + R2: B -> A; k2 * B; + end + """ + problem.model = SbmlModel.from_antimony(ant_model) + problem.add_observable("obs_a", formula="A", noise_formula=1.0) + problem.add_measurement("obs_a", time=20.0, measurement=5.0) + problem.add_parameter("k1", estimate=True, lb=1e-3, ub=1e3) + problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) + assert not problem.validate().has_errors() + + with pytest.raises(ValueError, match="did not pass linting"): + assignments_to_experiments(problem) From 9a43cca80d3c127a51388a3743d808752a520c39 Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Tue, 13 Oct 2026 16:19:57 +0100 Subject: [PATCH 08/12] trim scope of conversion --- petab/v2/converters.py | 58 +++++------ tests/v2/test_conversion.py | 193 ++++-------------------------------- 2 files changed, 46 insertions(+), 205 deletions(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index ebc74f0b..af4e710b 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -644,6 +644,8 @@ def assignments_to_experiments(problem: Problem) -> Problem: Measurements without an experiment ID are assigned to a new experiment :data:`DEFAULT_EXPERIMENT_ID`, if needed. + **Scope:** conversion is supported for a single piecewise function with exactly 3 arguments, where the 2nd argument is an inequality that can be re-arranged explicitly into `t />= ...`. Anything else raises a :class:`NotImplementedError`. + .. warning:: Switches during preequilibration are not supported. The problem @@ -652,6 +654,8 @@ def assignments_to_experiments(problem: Problem) -> Problem: :param problem: The PEtab problem to convert. This will not be modified. :returns: The converted PEtab problem. + :raises NotImplementedError: + If a time-dependent assignment is outside the scope described above. :raises ValueError: If the converted problem does not pass linting. """ @@ -690,32 +694,28 @@ def assignments_to_experiments(problem: Problem) -> Problem: pws = [pw for pw in expr.atoms(sp.Piecewise) if pw.has(TimeSymbol)] if not pws: continue - if len(pws) > 1 or len(pws[0].args) != 2: - raise NotImplementedError( - "Only assignments with a single time-dependent piecewise " - "function with a single condition are supported, but " - f"the assignment to `{assignment.getId()}` is `{expr}`." - ) - (pw,) = pws - (time,) = pw.atoms(TimeSymbol) - (value, switch), (otherwise, _) = pw.args target_id = assignment.getId() - - # `switch` is equivalent to `g > 0` (or `g >= 0`), so it holds after - # the switch if `g` increases with time, and before it otherwise - g = switch.gts - switch.lts - slope = sp.diff(g, time) - if slope.is_positive: - before, after = otherwise, value - elif slope.is_negative: - before, after = value, otherwise - else: + pw = pws[0] + (time,) = pw.atoms(TimeSymbol) + switch = pw.args[0].cond + if not ( + len(pws) == 1 + and len(pw.args) == 2 + and pw.args[1].cond == sp.true + and isinstance(switch, (sp.Gt, sp.Ge, sp.Lt, sp.Le)) + and not expr.subs(switch, sp.Dummy()).has(time) + and (slope := sp.diff(switch.gts - switch.lts, time)).is_number + and slope.is_nonzero + ): raise NotImplementedError( - "Cannot determine whether the condition " - f"`{switch}` of the assignment to `{target_id}` holds " - "before or after the switch." + "Only time dependence via a single 3-argument piecewise " + "function with a condition linear in time is supported, but " + f"the assignment to `{target_id}` is `{expr}`." ) - t_switch = sp.solve(g, time)[0] + # `switch` holds after the switch iff `gts - lts` increases with time + value, otherwise = (piece.expr for piece in pw.args) + before, after = (otherwise, value) if slope > 0 else (value, otherwise) + t_switch = sp.solve(switch.gts - switch.lts, time)[0] default = expr.subs(pw, before) after_expr = expr.subs(pw, after) @@ -747,10 +747,6 @@ def assignments_to_experiments(problem: Problem) -> Problem: f"`{experiment.id}` is `{t}`." ) t = float(t) - cur_before, cur_after = ( - default.subs(subs), - after_expr.subs(subs), - ) if period.is_preequilibration: # the value at time 0, which respects whether the @@ -763,13 +759,11 @@ def assignments_to_experiments(problem: Problem) -> Problem: t_end = periods[i + 1].time if i + 1 < len(periods) else sp.oo switch_value = ( - cur_after.subs(time, t) - if period.time < t < t_end - else None + after_expr.subs(subs) if period.time < t < t_end else None ) start_value = ( - cur_before if period.time < t else cur_after - ).subs(time, period.time) + default if period.time < t else after_expr + ).subs(subs) # The model already has the pre-switch value at the start of # the first period, and the switch can become the first # period if it only refers to parameter table symbols. diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index ef66d344..7e94601f 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -614,64 +614,19 @@ def test_benchmark_collection(problem_id): assert len(problem.measurements) -def test_assignments_to_experiments(): - problem = Problem() - ant_model = """ - model conversion - species A, B; - A = 10; - B = 0; - k1 = piecewise(1, time < 10, 2); - k2 = 0.5; - R1: A -> B; k1 * A; - R2: B -> A; k2 * B; - end - """ - problem.model = SbmlModel.from_antimony(ant_model) - - problem.add_observable("obs_a", formula="A", noise_formula=1.0) - problem.add_measurement("obs_a", time=0.0, measurement=10.0) - problem.add_measurement("obs_a", time=20.0, measurement=5.0) - problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - - converted = assignments_to_experiments(problem) - - # the value before the switch is kept in the model, so that only the - # switch itself needs a condition - assert converted.model.sbml_model.getParameter("k1").getValue() == 1.0 - - experiments_expected = pd.DataFrame( - data={ - v2.C.EXPERIMENT_ID: ["default_experiment"], - v2.C.TIME: [10.0], - v2.C.CONDITION_ID: ["cond_0"], - } - ) - - conditions_expected = pd.DataFrame( - data={ - v2.C.CONDITION_ID: ["cond_0"], - v2.C.TARGET_ID: ["k1"], - v2.C.TARGET_VALUE: [2.0], - } - ) - - assert converted.experiment_df.equals(experiments_expected) - assert converted.condition_df.equals(conditions_expected) - assert converted.measurement_df[v2.C.EXPERIMENT_ID].tolist() == [ - "default_experiment", - "default_experiment", - ] - - -def test_assignments_to_experiments_reversed(): +@pytest.mark.parametrize( + "condition, before, after", + [("time < 10", 1.0, 2.0), ("10 < time", 2.0, 1.0)], +) +def test_assignments_to_experiments(condition, before, after): + """Test that switches in either direction are converted.""" problem = Problem() - ant_model = """ + ant_model = f""" model conversion species A, B; A = 10; B = 0; - k1 = piecewise(1, 10 > time, 2); + k1 = piecewise(1, {condition}, 2); k2 = 0.5; R1: A -> B; k1 * A; R2: B -> A; k2 * B; @@ -688,7 +643,7 @@ def test_assignments_to_experiments_reversed(): # the value before the switch is kept in the model, so that only the # switch itself needs a condition - assert converted.model.sbml_model.getParameter("k1").getValue() == 1.0 + assert converted.model.sbml_model.getParameter("k1").getValue() == before experiments_expected = pd.DataFrame( data={ @@ -702,7 +657,7 @@ def test_assignments_to_experiments_reversed(): data={ v2.C.CONDITION_ID: ["cond_0"], v2.C.TARGET_ID: ["k1"], - v2.C.TARGET_VALUE: [2.0], + v2.C.TARGET_VALUE: [after], } ) @@ -886,62 +841,18 @@ def test_assignments_to_experiments_preequilibration(): @pytest.mark.parametrize( - "assignment", + "assignment, match", [ - "piecewise(0, time < 10, 1) + piecewise(0, time < 20, 1)", - "piecewise(0, time < 10, 1, time < 20, 2)", + ("piecewise(0, time < 10, 1) + piecewise(0, time < 20, 1)", "single"), + ("piecewise(0, time < 10, 1, time < 20, 2)", "single"), + ("piecewise(0, time == 10, 1)", "single"), + ("piecewise(0, time^2 < 10, 1)", "single"), + ("piecewise(time, time < 10, 1)", "single"), + ("piecewise(0, time < t_dose, 1)", "evaluate to a real number"), ], ) -def test_assignments_to_experiments_multiple_switches(assignment): - """Test that assignments with more than one switch are rejected.""" - problem = Problem() - ant_model = f""" - model conversion - species A, B; - A = 10; - B = 0; - k1 := {assignment}; - k2 = 0.5; - R1: A -> B; k1 * A; - R2: B -> A; k2 * B; - end - """ - problem.model = SbmlModel.from_antimony(ant_model) - - problem.add_observable("obs_a", formula="A", noise_formula=1.0) - problem.add_measurement("obs_a", time=0.0, measurement=10.0) - problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - - with pytest.raises(NotImplementedError, match="single time-dependent"): - assignments_to_experiments(problem) - - -@pytest.mark.parametrize( - "assignment, estimated, match", - [ - # depends on a species - ("piecewise(0, time < A, 1)", None, "evaluate to a real number"), - # switch direction depends on the sign of a species - ("piecewise(0, A * time < 5, 1)", None, "before or after the switch"), - # depends on an estimated parameter - ( - "piecewise(0, time < t_dose, 1)", - "t_dose", - "evaluate to a real number", - ), - # no real solution, and not monotonic in time - ( - "piecewise(0, time^2 + 1 < 0, 1)", - None, - "before or after the switch", - ), - ], -) -def test_assignments_to_experiments_non_numeric_switch( - assignment, estimated, match -): - """Test that switching times that are not real numbers, or switches - in an unknown direction, are rejected.""" +def test_assignments_to_experiments_unsupported(assignment, match): + """Test that assignments outside the supported scope are rejected.""" problem = Problem() ant_model = f""" model conversion @@ -960,75 +871,12 @@ def test_assignments_to_experiments_non_numeric_switch( problem.add_observable("obs_a", formula="A", noise_formula=1.0) problem.add_measurement("obs_a", time=0.0, measurement=10.0) problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - if estimated: - problem.add_parameter(estimated, estimate=True, lb=0, ub=10) + problem.add_parameter("t_dose", estimate=True, lb=0, ub=10) with pytest.raises(NotImplementedError, match=match): assignments_to_experiments(problem) -def test_assignments_to_experiments_switch_direction(): - """Test that `time < t` and `t < time` switch in opposite directions.""" - - def convert(assignment): - problem = Problem() - ant_model = f""" - model conversion - species A, B; - A = 10; - B = 0; - k1 := {assignment}; - k2 = 0.5; - R1: A -> B; k1 * A; - R2: B -> A; k2 * B; - end - """ - problem.model = SbmlModel.from_antimony(ant_model) - problem.add_observable("obs_a", formula="A", noise_formula=1.0) - problem.add_measurement("obs_a", time=20.0, measurement=5.0) - problem.add_parameter("k2", estimate=True, lb=1e-3, ub=1e3) - return assignments_to_experiments(problem) - - # k1 is 0 before and 1 after t=10 - converted_lt = convert("piecewise(0, time < 10, 1)") - # k1 is 1 before and 0 after t=10 - converted_gt = convert("piecewise(0, 10 < time, 1)") - - # the value before the switch is kept in the model - assert converted_lt.model.sbml_model.getParameter("k1").getValue() == 0.0 - assert converted_gt.model.sbml_model.getParameter("k1").getValue() == 1.0 - - experiments_expected = pd.DataFrame( - data={ - v2.C.EXPERIMENT_ID: ["default_experiment"], - v2.C.TIME: [10.0], - v2.C.CONDITION_ID: ["cond_0"], - } - ) - assert converted_lt.experiment_df.equals(experiments_expected) - assert converted_gt.experiment_df.equals(experiments_expected) - - # the switch sets k1 to the value after t=10 - assert converted_lt.condition_df.equals( - pd.DataFrame( - data={ - v2.C.CONDITION_ID: ["cond_0"], - v2.C.TARGET_ID: ["k1"], - v2.C.TARGET_VALUE: [1.0], - } - ) - ) - assert converted_gt.condition_df.equals( - pd.DataFrame( - data={ - v2.C.CONDITION_ID: ["cond_0"], - v2.C.TARGET_ID: ["k1"], - v2.C.TARGET_VALUE: [0.0], - } - ) - ) - - @pytest.mark.parametrize("t_switch", [0, 10]) @pytest.mark.parametrize( "condition, holds_at_switch, holds_before", @@ -1037,7 +885,6 @@ def convert(assignment): ("time <= {t}", True, True), ("time > {t}", False, False), ("time >= {t}", True, False), - ("{t} < time", False, False), ("{t} >= time", True, True), ], ) From 2900a70a893ca46b28108f62491ff468ae0788e3 Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Tue, 13 Oct 2026 16:26:32 +0100 Subject: [PATCH 09/12] fix ruff --- petab/v2/converters.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index af4e710b..8d6fc26e 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -644,7 +644,10 @@ def assignments_to_experiments(problem: Problem) -> Problem: Measurements without an experiment ID are assigned to a new experiment :data:`DEFAULT_EXPERIMENT_ID`, if needed. - **Scope:** conversion is supported for a single piecewise function with exactly 3 arguments, where the 2nd argument is an inequality that can be re-arranged explicitly into `t />= ...`. Anything else raises a :class:`NotImplementedError`. + **Scope:** conversion is supported for a single piecewise function with + exactly 3 arguments, where the 2nd argument is an inequality that can be + re-arranged explicitly into `t />= ...`. Anything else raises a + :class:`NotImplementedError`. .. warning:: From 60f805741fc32f1ca77561ab3ea413ef3794c61a Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Tue, 13 Oct 2026 16:44:56 +0100 Subject: [PATCH 10/12] not implemented for variable model params --- petab/v2/converters.py | 14 +++++++++++++- tests/v2/test_conversion.py | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index 8d6fc26e..b62d3ae6 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -646,7 +646,8 @@ def assignments_to_experiments(problem: Problem) -> Problem: **Scope:** conversion is supported for a single piecewise function with exactly 3 arguments, where the 2nd argument is an inequality that can be - re-arranged explicitly into `t />= ...`. Anything else raises a + re-arranged explicitly into `t />= ...`, and whose other symbols + are all constant model parameters. Anything else raises a :class:`NotImplementedError`. .. warning:: @@ -715,6 +716,17 @@ def assignments_to_experiments(problem: Problem) -> Problem: "function with a condition linear in time is supported, but " f"the assignment to `{target_id}` is `{expr}`." ) + if non_constant := sorted( + str(s) + for s in expr.free_symbols - {time} + if not (p := sbml_model.getParameter(str(s))) + or not p.getConstant() + ): + raise NotImplementedError( + "Only constant model parameters are supported as symbols in " + f"the assignment to `{target_id}`, but it refers to " + f"{non_constant}." + ) # `switch` holds after the switch iff `gts - lts` increases with time value, otherwise = (piece.expr for piece in pw.args) before, after = (otherwise, value) if slope > 0 else (value, otherwise) diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 7e94601f..0cbc4e86 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -849,6 +849,8 @@ def test_assignments_to_experiments_preequilibration(): ("piecewise(0, time^2 < 10, 1)", "single"), ("piecewise(time, time < 10, 1)", "single"), ("piecewise(0, time < t_dose, 1)", "evaluate to a real number"), + ("piecewise(0, time < A, 1)", "constant model parameters"), + ("A * piecewise(0, time < 10, 1)", "constant model parameters"), ], ) def test_assignments_to_experiments_unsupported(assignment, match): From 54cd742bbe3ee94e9c8bf3d135398ad2c5ca4b48 Mon Sep 17 00:00:00 2001 From: BSnelling Date: Wed, 14 Oct 2026 14:15:11 +0100 Subject: [PATCH 11/12] Apply batched suggestions from code review Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> --- petab/v2/converters.py | 37 ++++++++++++++++++++++++++++++++++++- tests/v2/test_conversion.py | 2 +- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index b62d3ae6..41af0421 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -654,7 +654,35 @@ def assignments_to_experiments(problem: Problem) -> Problem: Switches during preequilibration are not supported. The problem generated by this conversion has the assignment target keep its value - at time 0 throughout preequilibration. + Fixed parameters may change over the course of a single simulation, e.g., + as described by a v2 experiment with multiple periods. As v1 did not have + experiments, then these changes were + sometimes directly encoded in an SBML model as assignment rules with + piecewise functions. This method takes a v2 problem + (possibly converted from a v1 problem) and replaces those SBML assignment + rules with v2 experiments. + + .. note:: + + Measurements without an experiment ID are assigned to a new experiment + :data:`DEFAULT_EXPERIMENT_ID`, if needed. + + **Scope:** conversion is supported for SBML assignment rules that contain + a single piecewise function with exactly 3 arguments, where the 2nd + argument is an inequality that can be + re-arranged explicitly into `t />= ...`, and whose other symbols + are all truly constant model parameters. Anything else raises a + :class:`NotImplementedError`. + + .. warning:: + + As SBML assignment rules, the changes would occur during pre-equilibration. + As v2 experiments, the changes do not occur during pre-equilibration. + Hence, using this converter may affect the correctness of + your model's pre-equilibration. In this case, you could e.g. keep the + SBML assignment rules and/or add indicator variables to trigger them + correctly in the model for some/all pre-equilibration conditions. + :param problem: The PEtab problem to convert. This will not be modified. :returns: The converted PEtab problem. @@ -703,12 +731,19 @@ def assignments_to_experiments(problem: Problem) -> Problem: (time,) = pw.atoms(TimeSymbol) switch = pw.args[0].cond if not ( + # A single piecewise len(pws) == 1 + # with "conditional" and "otherwise" pieces and len(pw.args) == 2 + # and explicitly an "otherwise" piece, and pw.args[1].cond == sp.true + # with a supported inequality condition and isinstance(switch, (sp.Gt, sp.Ge, sp.Lt, sp.Le)) + # that involves the time variable, and not expr.subs(switch, sp.Dummy()).has(time) + # and that changes value at maximally a single time point and (slope := sp.diff(switch.gts - switch.lts, time)).is_number + # and this time point is identifiable. and slope.is_nonzero ): raise NotImplementedError( diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index 0cbc4e86..afffe8b5 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -647,7 +647,7 @@ def test_assignments_to_experiments(condition, before, after): experiments_expected = pd.DataFrame( data={ - v2.C.EXPERIMENT_ID: ["default_experiment"], + v2.C.EXPERIMENT_ID: [DEFAULT_EXPERIMENT_ID], v2.C.TIME: [10.0], v2.C.CONDITION_ID: ["cond_0"], } From 5720c99d8e0d3caad762d91e7c0ea49a6d1d58fa Mon Sep 17 00:00:00 2001 From: Branwen Snelling Date: Wed, 14 Oct 2026 14:22:58 +0100 Subject: [PATCH 12/12] updates to docstrings --- petab/v2/converters.py | 26 +++++++------------------- tests/v2/test_conversion.py | 11 +++++++---- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/petab/v2/converters.py b/petab/v2/converters.py index 41af0421..91e3a811 100644 --- a/petab/v2/converters.py +++ b/petab/v2/converters.py @@ -641,19 +641,6 @@ def _add_indicators_to_conditions(self) -> None: def assignments_to_experiments(problem: Problem) -> Problem: """Replace time-dependent piecewise assignments by experiment periods. - Measurements without an experiment ID are assigned to a new experiment - :data:`DEFAULT_EXPERIMENT_ID`, if needed. - - **Scope:** conversion is supported for a single piecewise function with - exactly 3 arguments, where the 2nd argument is an inequality that can be - re-arranged explicitly into `t />= ...`, and whose other symbols - are all constant model parameters. Anything else raises a - :class:`NotImplementedError`. - - .. warning:: - - Switches during preequilibration are not supported. The problem - generated by this conversion has the assignment target keep its value Fixed parameters may change over the course of a single simulation, e.g., as described by a v2 experiment with multiple periods. As v1 did not have experiments, then these changes were @@ -676,12 +663,13 @@ def assignments_to_experiments(problem: Problem) -> Problem: .. warning:: - As SBML assignment rules, the changes would occur during pre-equilibration. - As v2 experiments, the changes do not occur during pre-equilibration. - Hence, using this converter may affect the correctness of - your model's pre-equilibration. In this case, you could e.g. keep the - SBML assignment rules and/or add indicator variables to trigger them - correctly in the model for some/all pre-equilibration conditions. + As SBML assignment rules, the changes would occur during + pre-equilibration. As v2 experiments, the changes do not occur during + pre-equilibration. Hence, using this converter may affect the + correctness of your model's pre-equilibration. In this case, you could + e.g. keep the SBML assignment rules and/or add indicator variables to + trigger them correctly in the model for some/all pre-equilibration + conditions. :param problem: The PEtab problem to convert. This will not be modified. diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index afffe8b5..86ee3f69 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -11,7 +11,10 @@ from petab.v1.priors import Prior from petab.v2 import Problem from petab.v2.calculate import calculate_llh as calculate_llh_v2 -from petab.v2.converters import assignments_to_experiments +from petab.v2.converters import ( + DEFAULT_EXPERIMENT_ID, + assignments_to_experiments, +) from petab.v2.petab1to2 import petab1to2, v1v2_observable_df @@ -664,8 +667,8 @@ def test_assignments_to_experiments(condition, before, after): assert converted.experiment_df.equals(experiments_expected) assert converted.condition_df.equals(conditions_expected) assert converted.measurement_df[v2.C.EXPERIMENT_ID].tolist() == [ - "default_experiment", - "default_experiment", + DEFAULT_EXPERIMENT_ID, + DEFAULT_EXPERIMENT_ID, ] @@ -699,7 +702,7 @@ def test_assignments_to_experiments_nested(): # first period -- the pre-dose value is kept as a condition here experiments_expected = pd.DataFrame( data={ - v2.C.EXPERIMENT_ID: ["default_experiment"] * 2, + v2.C.EXPERIMENT_ID: [DEFAULT_EXPERIMENT_ID] * 2, v2.C.TIME: [0.0, 10.0], v2.C.CONDITION_ID: ["cond_0", "cond_1"], }