Skip to content

Commit 900f8b6

Browse files
authored
Update v2 condition table targetValue validation (#427)
After PEtab-dev/PEtab#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 #424.
1 parent de974ba commit 900f8b6

File tree

4 files changed

+104
-7
lines changed

4 files changed

+104
-7
lines changed

petab/v2/C.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,9 @@
285285
#: separator for multiple parameter values (bounds, observableParameters, ...)
286286
PARAMETER_SEPARATOR = ";"
287287

288+
#: The time symbol for use in any PEtab-specific mathematical expressions
289+
TIME_SYMBOL = "time"
290+
288291

289292
__all__ = [
290293
x

petab/v2/core.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,8 +2125,23 @@ def add_experiment(self, id_: str, *args):
21252125
is added to the last one.
21262126
21272127
:param id_: The experiment ID.
2128-
:param args: Timepoints and associated conditions:
2129-
``time_1, condition_id_1, time_2, condition_id_2, ...``.
2128+
:param args: Timepoints and associated conditions
2129+
(single condition ID as string or multiple condition IDs as lists
2130+
of strings).
2131+
2132+
:example:
2133+
>>> p = Problem()
2134+
>>> p.add_experiment(
2135+
... "experiment1",
2136+
... 1,
2137+
... "condition1",
2138+
... 2,
2139+
... ["condition2a", "condition2b"],
2140+
... )
2141+
>>> p.experiments[0] # doctest: +NORMALIZE_WHITESPACE
2142+
Experiment(id='experiment1', periods=[\
2143+
ExperimentPeriod(time=1.0, condition_ids=['condition1']), \
2144+
ExperimentPeriod(time=2.0, condition_ids=['condition2a', 'condition2b'])])
21302145
"""
21312146
if len(args) % 2 != 0:
21322147
raise ValueError(

petab/v2/lint.py

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"CheckUnusedConditions",
4444
"CheckPriorDistribution",
4545
"CheckUndefinedExperiments",
46+
"CheckInitialChangeSymbols",
4647
"lint_problem",
4748
"default_validation_tasks",
4849
]
@@ -736,6 +737,62 @@ def run(self, problem: Problem) -> ValidationIssue | None:
736737
return None
737738

738739

740+
class CheckInitialChangeSymbols(ValidationTask):
741+
"""
742+
Check that changes of any first period of any experiment only refers to
743+
allowed symbols.
744+
745+
The only allowed symbols are those that are present in the parameter table.
746+
"""
747+
748+
def run(self, problem: Problem) -> ValidationIssue | None:
749+
if not problem.experiments:
750+
return None
751+
752+
if not problem.conditions:
753+
return None
754+
755+
allowed_symbols = {p.id for p in problem.parameters}
756+
allowed_symbols.add(TIME_SYMBOL)
757+
# IDs of conditions that have already been checked
758+
valid_conditions = set()
759+
id_to_condition = {c.id: c for c in problem.conditions}
760+
761+
messages = []
762+
for experiment in problem.experiments:
763+
if not experiment.periods:
764+
continue
765+
766+
first_period = experiment.sorted_periods[0]
767+
for condition_id in first_period.condition_ids:
768+
if condition_id in valid_conditions:
769+
continue
770+
771+
# we assume that all referenced condition IDs are valid
772+
condition = id_to_condition[condition_id]
773+
774+
used_symbols = {
775+
str(sym)
776+
for change in condition.changes
777+
for sym in change.target_value.free_symbols
778+
}
779+
invalid_symbols = used_symbols - allowed_symbols
780+
if invalid_symbols:
781+
messages.append(
782+
f"Condition {condition.id} is applied at the start of "
783+
f"experiment {experiment.id}, and thus, its "
784+
f"target value expressions must only contain "
785+
f"symbols from the parameter table, or `time`. "
786+
"However, it contains additional symbols: "
787+
f"{invalid_symbols}. "
788+
)
789+
790+
if messages:
791+
return ValidationError("\n".join(messages))
792+
793+
return None
794+
795+
739796
class CheckPriorDistribution(ValidationTask):
740797
"""A task to validate the prior distribution of a PEtab problem."""
741798

@@ -1082,10 +1139,7 @@ def get_placeholders(
10821139
CheckValidParameterInConditionOrParameterTable(),
10831140
CheckUnusedExperiments(),
10841141
CheckUnusedConditions(),
1085-
# TODO: atomize checks, update to long condition table, re-enable
1086-
# TODO validate mapping table
1087-
CheckValidParameterInConditionOrParameterTable(),
1088-
CheckAllParametersPresentInParameterTable(),
1089-
CheckValidConditionTargets(),
10901142
CheckPriorDistribution(),
1143+
CheckInitialChangeSymbols(),
1144+
# TODO validate mapping table
10911145
]

tests/v2/test_lint.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,28 @@ def test_undefined_experiment_id_in_measurements():
8282
problem.measurements[0].experiment_id = "invalid_experiment_id"
8383
assert (error := check.run(problem)) is not None
8484
assert "not defined" in error.message
85+
86+
87+
def test_validate_initial_change_symbols():
88+
"""Test validation of symbols in target value expressions for changes
89+
applied at the start of an experiment."""
90+
problem = Problem()
91+
problem.model = SbmlModel.from_antimony("p1 = 1; p2 = 2")
92+
problem.add_experiment("e1", 0, "c1", 1, "c2")
93+
problem.add_condition("c1", p1="p2 + time")
94+
problem.add_condition("c2", p1="p2", p2="p1")
95+
problem.add_parameter("p1", nominal_value=1, estimate=False)
96+
problem.add_parameter("p2", nominal_value=2, estimate=False)
97+
98+
check = CheckInitialChangeSymbols()
99+
assert check.run(problem) is None
100+
101+
# removing `p1` from the parameter table is okay, as `c2` is never
102+
# used at the start of an experiment
103+
problem.parameter_tables[0].parameters.remove(problem["p1"])
104+
assert check.run(problem) is None
105+
106+
# removing `p2` is not okay, as it is used at the start of an experiment
107+
problem.parameter_tables[0].parameters.remove(problem["p2"])
108+
assert (error := check.run(problem)) is not None
109+
assert "contains additional symbols: {'p2'}" in error.message

0 commit comments

Comments
 (0)