assignments_to_experiments: fix lost switches, dropped changes and late evaluation of condition changes (#574)

Follows #573. Fixes in `assignments_to_experiments`, found while
reviewing #573's refactor and by differential testing against
simulation. Each one changes the converted problem, and each has a
regression test that fails without the fix. None was introduced by the
refactor.

## Fixes

- **A switch exactly at the start of a later period was lost** when that
period did not change any symbol of the assignment. With `k1 :=
piecewise(0, time < 10, 1)` and periods at 0 and 10, k1 stayed 0 after t
= 10. Likewise `time <= 0` after preequilibration. A later period now
also sets the value when it differs from the value at the end of the
previous period.
- **Condition changes of symbols still used elsewhere were dropped.**
With `k1 := dose * piecewise(0, time < 10, 1)` and a reaction that also
uses `dose`, the condition's `dose = 2` was removed, so the reaction
silently used the model value. Changes are now only dropped if nothing
else (the model's math or conversion factors, an observable, a condition
change) refers to the symbol.
- **A condition change was re-evaluated wherever a switch used it**,
with the values at the switch. PEtab applies a condition once, at the
start of its period, and all its changes simultaneously, with the values
from before the period. With `d1 = b1 + 0.5` at t = 2 and `b1 = 2` at t
= 3 (or `b1 = 2` in the same condition), a switch to `d1` at t = 5 used
2.5 instead of 1.7. Changes are now evaluated at the start of their
period; one whose value refers to something not known there, e.g. a
species, is rejected. As a side effect, switching times set to
expressions such as `t1 = t2 + 1` now convert instead of being rejected.
- **Time-dependent initial assignments were converted into switches**,
but SBML evaluates an initial assignment once, at the start of the
simulation, so `k1 = piecewise(0, time < 10, 1)` never switches. They
now stay in the model, as the docstring's scope ("assignment rules")
already said.
- **At preequilibration, a value referring to model parameters outside
the parameter table failed the lint** (`CheckInitialChangeSymbols`),
since preequilibration starts the experiment. Those parameters now take
their model values, as at the first period.
- **An assignment to a species or compartment crashed** with an
`AttributeError`; it now raises `NotImplementedError`.
- **A problem without time-dependent assignments was still changed**: an
experiment that only preequilibrates got an empty period at t = 0, and
periods were re-sorted. It is now returned as it was, after the lint.
- **A ramp's slope parameter could clash with PEtab IDs**, e.g. an
observable parameter `k1_slope`, since it was only checked against SBML
IDs. It is now named with `Problem.get_unused_id`.
- **A switching time that a condition set from a parameter fixed by the
parameter table was rejected**, e.g. `t1 = p + 1`, although `t1 := p +
1` in the assignment itself converts with `p`'s nominal value. Fixed
parameters now take their nominal values in condition changes too;
switching times that depend on estimated parameters are still rejected.
- **New condition values used the symbols parsed from SBML**, without
the assumptions of PEtab's symbols (`petab_symbol`, real), so they
compared unequal to the same expressions parsed by PEtab, e.g. after
reading the condition table back. They now use PEtab's symbols.

Test changes: `test_assignments_to_experiments` and
`_unused_condition_id` used an initial assignment (`k1 = ...`) to mean a
switching rule and now use an assignment rule (`k1 := ...`). `_validate`
relied on an estimated parameter with a time-dependent initial
assignment; it now uses an existing experiment `default_experiment`, to
which the measurements without an experiment ID would also be assigned,
which the final lint catches.

## Module

As suggested in review, `assignments_to_experiments` and its helpers now
live in their own module, `petab.v2.assignments_to_experiments` (moved
unchanged; still exported from `petab.v2.converters`), and their tests
in `tests/v2/test_assignments_to_experiments.py`.
`SbmlModel.get_referenced_ids()` and `ObservableTable.free_symbols` are
new, also from review.

## Checked

- `pytest tests/v2 tests/v1/test_petab.py tests/v1/test_sbml.py`: 266
passed; pre-commit clean.
- The benchmark problems Brannmark_JBC2010, Fujita_SciSignal2010,
Isensee_JCB2018 and Raimundez_PCB2020 (and Oliveira_NatCommun2021's
error) convert identically before and after.
- Differential testing against simulation: random problems with dose
sums, several and nested pieces, ramps, every inequality form, switching
times set by conditions (often exactly at period starts), symbols shared
with reactions, two converted assignments in one model, and condition
values that are expressions in other parameters. Each experiment of the
original and the converted problem is turned into SBML events with
`ExperimentsToSbmlConverter`, simulated in BasiCO, and compared (rtol
1e-6). On this PR's head, 2093 problems match and none is rejected; in 7
more, COPASI fails on the original model, and the converted one matches
it up to there. The same generator finds 59 mismatches in 300 problems
on `main`. Not covered: preequilibration, experiments that start at t ≠
0, and first periods with expression values (only parameter table
symbols are allowed there).
- A second pass with an independent reference simulator (SciPy,
integrating piecewise between all switching times and period starts; it
agrees with BasiCO on 94 cross-checked simulations), covering what the
first could not: experiments that start at t = -5, 2 or 5,
preequilibration (with the documented semantics: converted assignments
hold their value at time 0), parameter table entries whose estimated or
fixed nominal values differ from the model's (in values, switching times
and condition values), condition values that refer to parameter table
symbols at the start and to a species in unrelated changes. Each problem
also checks that the input stays unchanged, that conversion is
deterministic and idempotent, and that the converted condition and
experiment tables survive a round trip through their DataFrames. On this
PR's head, 1415 of 1500 problems match and pass all checks; the other 85
are rejected because a switching time depends on an estimated parameter,
as intended. On `main`, the same pass finds 21 mismatches and 16 lint
failures of the converted problem in 200 problems.
- Where BasiCO disagrees with the reference simulator on an original
problem (a switch of the assignment rule exactly at the time of a
period's event, which COPASI mis-simulates; moving the switch by 0.001
removes the disagreement), the reference simulator agrees with the
converted problem.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: FFroehlich

CHANGELOG.md

@@ -59,6 +59,15 @@
   that PEtab condition changes are applied before its assignments.
   Previously, their order was undefined
   (by @dweindl in https://github.com/PEtab-dev/libpetab-python/pull/562)
+* Several fixes to `assignments_to_experiments`: keep a switch at the start
+  of a later period, keep the condition changes of symbols that the model
+  still uses elsewhere, evaluate a condition change once at the start of its
+  period, leave time-dependent initial assignments in the model, reject
+  assignments to species or compartments, leave problems without
+  time-dependent assignments unchanged, give a ramp's slope parameter an
+  unused ID, and accept switching times that conditions set from parameters
+  fixed by the parameter table
+  (by @FFroehlich in https://github.com/PEtab-dev/libpetab-python/pull/574)
 
 **Features**
 * `petab1to2` and `petab_files_1to2` can skip validation via `validate=False`

doc/modules.rst

@@ -32,6 +32,7 @@ API Reference
    petab.v1.yaml
    petab.v2
    petab.v2.C
+   petab.v2.assignments_to_experiments
    petab.v2.converters
    petab.v2.core
    petab.v2.experiments

petab/v1/models/sbml_model.py

@@ -262,6 +262,37 @@ def get_valid_ids_for_condition_table(self) -> Iterable[str]:
     def symbol_allowed_in_observable_formula(self, id_: str) -> bool:
         return self.sbml_model.getElementBySId(id_) or id_ == "time"
 
+    def get_referenced_ids(self) -> set[str]:
+        """Get the IDs that the model refers to.
+
+        These are the IDs in the math of any model element (e.g., kinetic
+        laws, rules, initial assignments, events), and those referred to as
+        conversion factors. IDs that are only defined, but not referred to,
+        are not included.
+        """
+        ids = set()
+        for element in self.sbml_model.getListOfAllElements():
+            if not (hasattr(element, "isSetMath") and element.isSetMath()):
+                continue
+            nodes = [element.getMath()]
+            while nodes:
+                node = nodes.pop()
+                if node.getType() == libsbml.AST_NAME:
+                    ids.add(node.getName())
+                nodes += [
+                    node.getChild(i) for i in range(node.getNumChildren())
+                ]
+        # conversion factors refer to parameters by attribute, not in math
+        ids |= {
+            element.getConversionFactor()
+            for element in [
+                self.sbml_model,
+                *self.sbml_model.getListOfSpecies(),
+            ]
+            if element.isSetConversionFactor()
+        }
+        return ids
+
     def is_valid(self) -> bool:
         return is_sbml_consistent(self.sbml_model.getSBMLDocument())