Skip to content

Commit e5b1b36

Browse files
dweindlclaude
andcommitted
Auto-assign priority to a single unprioritized event
`ExperimentsToSbmlConverter` now assigns a priority to a pre-existing model event without priority if it is the only such event, so that PEtab condition changes are applied before its assignments. The assigned priority is lower than all other priorities in the model, so no ties are introduced. This only applies if `default_priority` is not provided. Previously, the order of such events relative to the PEtab events was undefined (e.g., PEtab test suite case 0030). References #526 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 73b680c commit e5b1b36

2 files changed

Lines changed: 154 additions & 4 deletions

File tree

‎petab/v2/converters.py‎

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import logging
56
import warnings
67
from copy import deepcopy
78

@@ -22,6 +23,8 @@
2223

2324
__all__ = ["ExperimentsToSbmlConverter"]
2425

26+
logger = logging.getLogger(__name__)
27+
2528

2629
class ExperimentsToSbmlConverter:
2730
"""Convert PEtab experiments to SBML.
@@ -34,6 +37,10 @@ class ExperimentsToSbmlConverter:
3437
are applied before any pre-existing assignments.
3538
This requires that all event priorities in the original model are numeric
3639
constants.
40+
If exactly one pre-existing event has no priority, it is assigned a
41+
priority lower than all other priorities in the model.
42+
If multiple events have no priority, their order relative to the PEtab
43+
events is undefined, unless ``default_priority`` is provided.
3744
3845
The PEtab problem must not contain any identifiers starting with
3946
``_petab``.
@@ -73,6 +80,11 @@ def __init__(self, problem: Problem, default_priority: float = None):
7380
See SBML specs for details.
7481
To ensure that the PEtab condition-start-events are executed before
7582
any other events, all events should have a priority set.
83+
Note that, according to the SBML specs, events with equal
84+
priorities that are executed simultaneously must be executed in
85+
random order.
86+
If ``None``, a priority is only assigned automatically if exactly
87+
one pre-existing event has no priority.
7688
"""
7789
if len(problem.models) > 1:
7890
# https://github.com/PEtab-dev/libpetab-python/issues/392
@@ -121,14 +133,13 @@ def _preprocess(self) -> None:
121133
# Apply default priority to all events that do not have a priority
122134
if self._default_priority is not None:
123135
for event in model.getListOfEvents():
124-
if (
125-
not event.getPriority()
126-
or event.getPriority().getMath() is None
127-
):
136+
if not self._has_priority(event):
128137
priority = event.createPriority()
129138
priority.setMath(
130139
libsbml.parseL3Formula(str(self._default_priority))
131140
)
141+
else:
142+
self._auto_assign_priority()
132143

133144
# Collect event priorities
134145
event_priorities = {
@@ -186,6 +197,54 @@ def _preprocess(self) -> None:
186197
stacklevel=1,
187198
)
188199

200+
@staticmethod
201+
def _has_priority(event: libsbml.Event) -> bool:
202+
"""Check whether the given event has a priority expression."""
203+
return (
204+
event.isSetPriority() and event.getPriority().getMath() is not None
205+
)
206+
207+
def _auto_assign_priority(self) -> None:
208+
"""Assign a priority to a single unprioritized pre-existing event.
209+
210+
If exactly one pre-existing event has no priority, and all other
211+
events have constant priorities, the unprioritized event is assigned
212+
a priority lower than any other priority in the model.
213+
Without priority, the order of this event with respect to all other
214+
events is undefined, so assigning a priority only selects one of the
215+
admissible orders. A distinct priority cannot create ties, which
216+
would require random execution order (SBML L3V2 §4.12.3). This allows
217+
executing the PEtab events before this event.
218+
219+
If more than one event has no priority, no priority is assigned,
220+
since this would impose an arbitrary order on them if they are
221+
executed simultaneously.
222+
"""
223+
events = list(self._model.getListOfEvents())
224+
unprioritized = [ev for ev in events if not self._has_priority(ev)]
225+
if len(unprioritized) != 1:
226+
return
227+
228+
priorities = []
229+
for ev in events:
230+
if not self._has_priority(ev):
231+
continue
232+
priority = sbml_math_to_sympy(ev.getPriority())
233+
if priority.free_symbols:
234+
# handled in _preprocess
235+
return
236+
priorities.append(float(priority))
237+
238+
value = min(priorities) - 1 if priorities else 0.0
239+
event = unprioritized[0]
240+
priority = event.createPriority()
241+
set_math(priority, value)
242+
logger.debug(
243+
f"Assigned priority {value} to event `{event.getId()}` to "
244+
"ensure that PEtab condition changes are applied before this "
245+
"event's assignments."
246+
)
247+
189248
def convert(self) -> Problem:
190249
"""Convert the PEtab experiments to SBML events.
191250

‎tests/v2/test_converters.py‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
from math import inf
22

33
import pandas as pd
4+
import pytest
5+
from sbmlmath import sbml_math_to_sympy
46

57
from petab.v2 import Change, Condition, Experiment, ExperimentPeriod, Problem
68
from petab.v2.converters import ExperimentsToSbmlConverter
@@ -232,3 +234,92 @@ def test_simulate_experiment_to_events():
232234

233235
for col in df_expected.columns:
234236
assert (df_expected[col] == df_actual[col]).all()
237+
238+
239+
def _event_priorities(sbml_model) -> dict[str, float | None]:
240+
"""Get the numeric priorities of all events in the given model."""
241+
return {
242+
ev.getId(): float(sbml_math_to_sympy(ev.getPriority()))
243+
if ev.isSetPriority() and ev.getPriority().getMath() is not None
244+
else None
245+
for ev in sbml_model.getListOfEvents()
246+
}
247+
248+
249+
def _problem_0030(model_events: str) -> Problem:
250+
"""PEtab test suite case 0030 with the given model events."""
251+
ant_model = f"""
252+
compartment C = 4
253+
species S in C = 3
254+
p = 1
255+
S' = p
256+
{model_events}
257+
"""
258+
problem = Problem()
259+
problem.model = SbmlModel.from_antimony(ant_model)
260+
problem.add_condition("condition1", S=2)
261+
problem.add_condition("condition2", S="S + C", C=8)
262+
problem.add_experiment("experiment1", 0, "condition1", 10, "condition2")
263+
return problem
264+
265+
266+
E0 = "E0: at time >= 10, fromTrigger=false: C = C * 2"
267+
E0_PRIO = "E0: at time >= 10, priority=-2, fromTrigger=false: C = C * 2"
268+
E1 = "E1: at time >= 20, fromTrigger=false: C = C"
269+
E1_PRIO = "E1: at time >= 20, priority=5, fromTrigger=false: C = C"
270+
PETAB_EVENT = "_petab_event_experiment1_1"
271+
272+
273+
@pytest.mark.parametrize(
274+
"model_events,expected",
275+
[
276+
# single unprioritized event
277+
(E0, {"E0": 0, PETAB_EVENT: 1}),
278+
# one unprioritized event, others prioritized
279+
(f"{E0}\n{E1_PRIO}", {"E0": 4, "E1": 5, PETAB_EVENT: 6}),
280+
# all events prioritized: unchanged
281+
(f"{E0_PRIO}\n{E1_PRIO}", {"E0": -2, "E1": 5, PETAB_EVENT: 6}),
282+
# multiple unprioritized events: no automatic assignment
283+
(f"{E0}\n{E1}", {"E0": None, "E1": None, PETAB_EVENT: None}),
284+
# no model events
285+
("", {PETAB_EVENT: None}),
286+
],
287+
)
288+
def test_auto_event_priority(model_events, expected):
289+
"""Test automatic priority assignment for pre-existing events."""
290+
problem = _problem_0030(model_events)
291+
orig_priorities = _event_priorities(problem.model.sbml_model)
292+
293+
converted = ExperimentsToSbmlConverter(problem).convert()
294+
295+
assert _event_priorities(converted.model.sbml_model) == expected
296+
# the original problem is not modified
297+
assert _event_priorities(problem.model.sbml_model) == orig_priorities
298+
299+
300+
def test_auto_event_priority_empty_priority():
301+
"""A priority without math is treated as unset."""
302+
problem = _problem_0030(E0)
303+
problem.model.sbml_model.getEvent("E0").createPriority()
304+
305+
converted = ExperimentsToSbmlConverter(problem).convert()
306+
307+
assert _event_priorities(converted.model.sbml_model) == {
308+
"E0": 0,
309+
PETAB_EVENT: 1,
310+
}
311+
312+
313+
def test_explicit_default_priority():
314+
"""An explicit default priority takes precedence."""
315+
problem = _problem_0030(f"{E0}\n{E1}")
316+
317+
converted = ExperimentsToSbmlConverter(
318+
problem, default_priority=3
319+
).convert()
320+
321+
assert _event_priorities(converted.model.sbml_model) == {
322+
"E0": 3,
323+
"E1": 3,
324+
PETAB_EVENT: 4,
325+
}

0 commit comments

Comments
 (0)