From 4df1ecb3b7e3e6df0dcc3c15c25ba0ecbe34184f Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 21 Aug 2025 17:09:50 +0200 Subject: [PATCH] Update v2 condition table `targetValue` validation After https://github.com/PEtab-dev/PEtab/pull/645, any condition that is used as a first condition cannot refer to any symbols other then the parameters listed in the parameter table, or `time'. Closes https://github.com/PEtab-dev/libpetab-python/issues/424. --- petab/v2/C.py | 3 ++ petab/v2/core.py | 19 +++++++++++-- petab/v2/lint.py | 64 +++++++++++++++++++++++++++++++++++++++---- tests/v2/test_lint.py | 25 +++++++++++++++++ 4 files changed, 104 insertions(+), 7 deletions(-) diff --git a/petab/v2/C.py b/petab/v2/C.py index 5bb73980..e680450e 100644 --- a/petab/v2/C.py +++ b/petab/v2/C.py @@ -285,6 +285,9 @@ #: separator for multiple parameter values (bounds, observableParameters, ...) PARAMETER_SEPARATOR = ";" +#: The time symbol for use in any PEtab-specific mathematical expressions +TIME_SYMBOL = "time" + __all__ = [ x diff --git a/petab/v2/core.py b/petab/v2/core.py index e886be8b..0f55ea8e 100644 --- a/petab/v2/core.py +++ b/petab/v2/core.py @@ -2125,8 +2125,23 @@ def add_experiment(self, id_: str, *args): is added to the last one. :param id_: The experiment ID. - :param args: Timepoints and associated conditions: - ``time_1, condition_id_1, time_2, condition_id_2, ...``. + :param args: Timepoints and associated conditions + (single condition ID as string or multiple condition IDs as lists + of strings). + + :example: + >>> p = Problem() + >>> p.add_experiment( + ... "experiment1", + ... 1, + ... "condition1", + ... 2, + ... ["condition2a", "condition2b"], + ... ) + >>> p.experiments[0] # doctest: +NORMALIZE_WHITESPACE + Experiment(id='experiment1', periods=[\ +ExperimentPeriod(time=1.0, condition_ids=['condition1']), \ +ExperimentPeriod(time=2.0, condition_ids=['condition2a', 'condition2b'])]) """ if len(args) % 2 != 0: raise ValueError( diff --git a/petab/v2/lint.py b/petab/v2/lint.py index 6bfcfe50..4d864b57 100644 --- a/petab/v2/lint.py +++ b/petab/v2/lint.py @@ -43,6 +43,7 @@ "CheckUnusedConditions", "CheckPriorDistribution", "CheckUndefinedExperiments", + "CheckInitialChangeSymbols", "lint_problem", "default_validation_tasks", ] @@ -736,6 +737,62 @@ def run(self, problem: Problem) -> ValidationIssue | None: return None +class CheckInitialChangeSymbols(ValidationTask): + """ + Check that changes of any first period of any experiment only refers to + allowed symbols. + + The only allowed symbols are those that are present in the parameter table. + """ + + def run(self, problem: Problem) -> ValidationIssue | None: + if not problem.experiments: + return None + + if not problem.conditions: + return None + + allowed_symbols = {p.id for p in problem.parameters} + allowed_symbols.add(TIME_SYMBOL) + # IDs of conditions that have already been checked + valid_conditions = set() + id_to_condition = {c.id: c for c in problem.conditions} + + messages = [] + for experiment in problem.experiments: + if not experiment.periods: + continue + + first_period = experiment.sorted_periods[0] + for condition_id in first_period.condition_ids: + if condition_id in valid_conditions: + continue + + # we assume that all referenced condition IDs are valid + condition = id_to_condition[condition_id] + + used_symbols = { + str(sym) + for change in condition.changes + for sym in change.target_value.free_symbols + } + invalid_symbols = used_symbols - allowed_symbols + if invalid_symbols: + messages.append( + f"Condition {condition.id} is applied at the start of " + f"experiment {experiment.id}, and thus, its " + f"target value expressions must only contain " + f"symbols from the parameter table, or `time`. " + "However, it contains additional symbols: " + f"{invalid_symbols}. " + ) + + if messages: + return ValidationError("\n".join(messages)) + + return None + + class CheckPriorDistribution(ValidationTask): """A task to validate the prior distribution of a PEtab problem.""" @@ -1082,10 +1139,7 @@ def get_placeholders( CheckValidParameterInConditionOrParameterTable(), CheckUnusedExperiments(), CheckUnusedConditions(), - # TODO: atomize checks, update to long condition table, re-enable - # TODO validate mapping table - CheckValidParameterInConditionOrParameterTable(), - CheckAllParametersPresentInParameterTable(), - CheckValidConditionTargets(), CheckPriorDistribution(), + CheckInitialChangeSymbols(), + # TODO validate mapping table ] diff --git a/tests/v2/test_lint.py b/tests/v2/test_lint.py index 1013670f..7eb6dc91 100644 --- a/tests/v2/test_lint.py +++ b/tests/v2/test_lint.py @@ -82,3 +82,28 @@ def test_undefined_experiment_id_in_measurements(): problem.measurements[0].experiment_id = "invalid_experiment_id" assert (error := check.run(problem)) is not None assert "not defined" in error.message + + +def test_validate_initial_change_symbols(): + """Test validation of symbols in target value expressions for changes + applied at the start of an experiment.""" + problem = Problem() + problem.model = SbmlModel.from_antimony("p1 = 1; p2 = 2") + problem.add_experiment("e1", 0, "c1", 1, "c2") + problem.add_condition("c1", p1="p2 + time") + problem.add_condition("c2", p1="p2", p2="p1") + problem.add_parameter("p1", nominal_value=1, estimate=False) + problem.add_parameter("p2", nominal_value=2, estimate=False) + + check = CheckInitialChangeSymbols() + assert check.run(problem) is None + + # removing `p1` from the parameter table is okay, as `c2` is never + # used at the start of an experiment + problem.parameter_tables[0].parameters.remove(problem["p1"]) + assert check.run(problem) is None + + # removing `p2` is not okay, as it is used at the start of an experiment + problem.parameter_tables[0].parameters.remove(problem["p2"]) + assert (error := check.run(problem)) is not None + assert "contains additional symbols: {'p2'}" in error.message