Merge branch 'main' into goodness_of_fit

Author: plakrisenko

petab/petablint.py

@@ -12,8 +12,8 @@
 from jsonschema.exceptions import ValidationError as SchemaValidationError
 
 import petab.v1 as petab
+from petab.v1 import validate_yaml_semantics, validate_yaml_syntax
 from petab.v1.C import FORMAT_VERSION
-from petab.v1.yaml import validate
 from petab.versions import get_major_version
 
 logger = logging.getLogger(__name__)
@@ -159,7 +159,7 @@ def main():
 
     if args.yaml_file_name:
         try:
-            validate(args.yaml_file_name)
+            validate_yaml_syntax(args.yaml_file_name)
         except SchemaValidationError as e:
             path = ""
             if e.absolute_path:
@@ -181,6 +181,8 @@ def main():
 
         match get_major_version(args.yaml_file_name):
             case 1:
+                validate_yaml_semantics(args.yaml_file_name)
+
                 if petab.is_composite_problem(args.yaml_file_name):
                     # TODO: further checking:
                     #  https://github.com/ICB-DCM/PEtab/issues/191

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

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(

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
 ]

tests/v2/test_core.py

@@ -1,3 +1,4 @@
+import subprocess
 import tempfile
 from pathlib import Path
 
@@ -666,3 +667,43 @@ def test_generate_path():
     else:
         assert gp(Path("foo"), "/bar") == "/bar/foo"
         assert gp("/foo", "bar") == "/foo"
+
+
+def test_petablint_v2(tmpdir):
+    """Test that petablint runs on a valid v2 problem without errors."""
+    problem = Problem()
+    problem.model = SbmlModel.from_antimony("""
+    model conversion
+      species A, B;
+      A = 10;
+      B = 0;
+      k1 = 1;
+      k2 = 0.5;
+      R1: A -> B; k1 * A;
+      R2: B -> A; k2 * B;
+    end
+    """)
+    problem.add_observable("obs_A", "A", noise_formula="sd_A")
+    problem.add_parameter(
+        "k1", estimate=True, lb=1e-5, ub=1e5, nominal_value=1
+    )
+    problem.add_parameter(
+        "k2", estimate=True, lb=1e-5, ub=1e5, nominal_value=0.5
+    )
+    problem.add_parameter(
+        "sd_A", estimate=True, lb=0.01, ub=10, nominal_value=1
+    )
+    problem.add_measurement(
+        "obs_A", time=10, measurement=2.5, experiment_id=""
+    )
+    assert problem.validate() == []
+
+    problem.config = ProblemConfig(filepath="problem.yaml")
+    problem.models[0].rel_path = "model.xml"
+    problem.parameter_tables[0].rel_path = "parameters.tsv"
+    problem.observable_tables[0].rel_path = "observables.tsv"
+    problem.measurement_tables[0].rel_path = "measurements.tsv"
+    problem.to_files(Path(tmpdir))
+
+    result = subprocess.run(["petablint", str(Path(tmpdir, "problem.yaml"))])  # noqa: S603,S607
+    assert result.returncode == 0

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