Skip to content

assignments_to_experiments: convert assignments that switch more than once - #556

Merged
FFroehlich merged 3 commits into
mainfrom
claude/assignments-to-experiments-several-switches
Oct 19, 2026
Merged

FFroehlich merged 3 commits into
mainfrom
claude/assignments-to-experiments-several-switches

Conversation

@FFroehlich

@FFroehlich FFroehlich commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Extends assignments_to_experiments (#527) to assignments that switch more than once.

Why. Of the benchmark-collection problems with time-dependent piecewise assignments, main converts two (Isensee_JCB2018, Raimundez_PCB2020) and rejects three. Two of those have an exact form as experiment periods:

  • Brannmark_JBC2010, a sum of doses given at different times:
    insulin := insulin_dose_1 * piecewise(0, time - insulin_time_1 < 0, 1) + insulin_dose_2 * piecewise(0, time - insulin_time_2 < 0, 1)
  • Oliveira_NatCommun2021, phases one after another as nested piecewise functions:
    beta := piecewise(beta_0, time < t_1, piecewise(beta_1, time < t_2, ...))

What. An assignment may now depend on time through any number of piecewise conditions — in a sum, in several pieces, or nested — within the scope agreed in #527, applied per condition:

  • every condition is a single inequality that can be re-arranged into t </<=/>/>= ... (no &&/||, no equality, linear in time with a non-zero slope);
  • every time-dependent piecewise function has an explicit "otherwise" piece;
  • time enters only through those conditions;
  • every switching time evaluates to a real number in each experiment: it may depend on constant model parameters, on parameters fixed in the parameter table, or on parameters the condition table sets, but not on estimated parameters;
  • all other symbols are truly constant model parameters.

The value on each interval between switches is the assignment with every condition replaced by whether it holds there. A switch that changes the value becomes a period; one that does not (e.g. two phases with equal values) adds nothing.

The value before every switch follows from each condition's slope and is kept in the model. If it refers to parameter table symbols (e.g. an estimated d1 in piecewise(d1, time < 10, d2)), it is also set at the start of the first period, since the model can only hold a number for it. (On main, such an assignment fails with a TypeError.)

Switching times are checked up front, in _supported_time_switches: one that depends on an estimated parameter is rejected with an error naming those parameters. Otherwise, the symbols of a switching time that no condition sets take their value from the parameter table, if fixed there, or else from the model. On main, a switching time given by a constant model parameter is rejected. A condition that sets a switching time to an expression in estimated parameters is still rejected per experiment.

Everything that converts on main converts as before: all existing tests pass unchanged except in test_assignments_to_experiments_unsupported, where the two cases asserting that these two shapes are rejected are replaced by a conjunction (time < 10 && time > 5), still rejected, and the estimated switching time now expects the new error. The docstring's scope is updated accordingly; the preequilibration warning and the final lint are unchanged.

Checked

  • New tests: a sum of doses, several pieces, nested pieces, a switch that does not change the value, and condition-specific doses as in Brannmark (each experiment gets its own switching times and amounts); switching times given by a constant model parameter, a parameter fixed in the parameter table, or the condition table; a condition that sets a switching time to an estimated parameter (rejected); a pre-switch value that refers to estimated parameters.

  • On the benchmark problems (assignments_to_experiments after petab.v2.Problem.from_yaml, then simulated with AMICI's PEtab v2 importer at the nominal parameters, atol 1e-10):

    problem main this branch
    Brannmark_JBC2010 rejected converted (17 periods); llh −141.889201 → −141.889187
    Oliveira_NatCommun2021 rejected (shape) rejected: its switching times t_1, t_2 are estimated, which no period time can be; the error names them
    Fujita_SciSignal2010 rejected rejected: EGF_rate*time puts time in a value
    Isensee_JCB2018 converted, Δllh −1.8e-3 unchanged
    Raimundez_PCB2020 converted, Δllh 4.7e-5 unchanged (simulated with ExperimentsToSbmlConverter: write one flat initial assignment per target #546)

    So Oliveira does not become convertible — it now fails for the reason that actually prevents it, rather than for its shape.

    The fixed switching times and the pre-switch value were added after the simulations above. They leave the converted Brannmark, Isensee and Raimundez problems identical (experiment table, condition table, SBML), so those log-likelihoods still hold.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MNJePorYs9zmPYMam6g4EP

@codecov-commenter

codecov-commenter commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.07%. Comparing base (8bbe776) to head (79aa38e).

Files with missing lines Patch % Lines
petab/v2/converters.py 86.00% 3 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #556   +/-   ##
=======================================
  Coverage   78.07%   78.07%           
=======================================
  Files          68       68           
  Lines        7793     7823   +30     
  Branches     1389     1399   +10     
=======================================
+ Hits         6084     6108   +24     
- Misses       1224     1227    +3     
- Partials      485      488    +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@FFroehlich
FFroehlich marked this pull request as ready for review October 16, 2026 07:47
@FFroehlich
FFroehlich requested a review from a team as a code owner October 16, 2026 07:47
@FFroehlich
FFroehlich force-pushed the claude/assignments-to-experiments-several-switches branch from be9abb1 to f12ba01 Compare October 16, 2026 07:47
@FFroehlich
FFroehlich added this pull request to stack #558 October 16, 2026 09:19

@dilpath dilpath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks neat! Thanks.

I agree Oliveira can't be done. For Fujita it could be possible like
Current:

EGF = EGF_rate * time + EGF_0    if    time <= EGF_end
      0                          otherwise

Possible solution:

indicator = 1    if    time <= EGF_end
            0          otherwise

EGF = (EGF_rate * time + EGF_0) * indicator

But this requires a bit more code to create new SBML assignment rules for indicator variables of piecewises.

Comment thread petab/v2/converters.py
@dilpath

dilpath commented Oct 16, 2026

Copy link
Copy Markdown
Member

Regarding #557, do you prefer that solution or the indicator suggestion above? I think the indicator method is more generally applicable, i.e. it does not require a linear dependence on time.

@FFroehlich

Copy link
Copy Markdown
Collaborator Author

Regarding #557, do you prefer that solution or the indicator suggestion above? I think the indicator method is more generally applicable, i.e. it does not require a linear dependence on time.

I actually do like the indicator suggestion better!

@FFroehlich

Copy link
Copy Markdown
Collaborator Author

Regarding #557, do you prefer that solution or the indicator suggestion above? I think the indicator method is more generally applicable, i.e. it does not require a linear dependence on time.

I actually do like the indicator suggestion better!

Thinking about this again, I am not entirely happy with either solution and the solution in #557 is actually closer to the goal we are trying to achieve: the PEtab owns the experiment definition and the model owns the description of the biology.

… once

The conversion accepted a single piecewise function with one switch. Time-dependent inputs in
the benchmark collection also come as a sum of doses given at different times
(Brannmark_JBC2010: `d1 * piecewise(0, t < t1, 1) + d2 * piecewise(0, t < t2, 1)`) and as phases
one after another (Oliveira_NatCommun2021: nested piecewise functions), which both have an exact
form as experiment periods.

An assignment may now depend on time through any number of piecewise conditions -- in a sum,
in several pieces, or nested -- as long as each condition is a single inequality linear in time
(the scope agreed in #527, applied per condition), each time-dependent piecewise function has an
explicit otherwise piece, and every switching time evaluates to a real number in each experiment.
The value on each interval between switches is the assignment with every condition replaced by
whether it holds there; a switch that changes the value becomes a period, one that does not is
dropped. A single switch converts exactly as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MNJePorYs9zmPYMam6g4EP
@FFroehlich
FFroehlich force-pushed the claude/assignments-to-experiments-several-switches branch from f12ba01 to 93e8bf9 Compare October 19, 2026 08:29
FFroehlich and others added 2 commits October 19, 2026 10:21
…ated ones early

A switching time that depends on a constant model parameter, or on a
parameter fixed by the parameter table, now takes that value unless a
condition sets it; such switching times were rejected before.

A switching time that depends on an estimated parameter is now rejected in
`_supported_time_switches`, naming those parameters, rather than per
experiment with the generic "evaluate to a real number" error. That error
remains for a switching time that a condition sets to an expression in
estimated parameters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… parameter table

The value before every switch is written into the SBML model for the
model's parameter values. That number is only correct for symbols that are
not in the parameter table, so the change at the start of the first period
may only be left out if the pre-switch value does not refer to parameter
table symbols. Otherwise, an estimated `d1` in
`piecewise(d1, time < 10, d2)` would be ignored before the switch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@FFroehlich
FFroehlich merged commit da74567 into main Oct 19, 2026
13 checks passed
@FFroehlich
FFroehlich deleted the claude/assignments-to-experiments-several-switches branch October 19, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants