Repository navigation
ExperimentsToSbmlConverter: keep per-experiment constants in the condition table - #579
Open
FFroehlich wants to merge 2 commits into
Open
FFroehlich wants to merge 2 commits into
FFroehlich wants to merge 2 commits into
Conversation
…n table A change that sets a constant model parameter to a number at the start of an experiment, where nothing else assigns that parameter, is constant per experiment, like a column of a PEtab v1 condition table. Encoding it in the model added a branch per experiment to the parameter's initial assignment and an indicator parameter per experiment. Froehlich_CellSystems2018 has 9169 experiments that each set 145 such parameters: 145 piecewise initial assignments with about 9169 branches each, which AMICI takes hours to import. These changes now stay as conditions of the experiment's kept periods (both of them with pre-equilibration, since the value persists), and an experiment gets an indicator only if it has a change encoded in the model. AMICI already sets every remaining condition target as a fixed parameter per experiment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgN92Vr4NPT21RjToXf6GS
… too A species set to a number at the start of an experiment, and nowhere else, is a per-experiment constant as well: Froehlich_CellSystems2018 sets the initial amounts of 23 species per cell line, which stayed piecewise initial assignments with ~9169 branches each. Such a species now gets a parameter `_petab_initial_value_<id>` holding its initial value and the initial assignment `<id> = _petab_initial_value_<id>`, and its changes are kept as changes of that parameter. Only species whose initial value is a number (after substituting compartment sizes that nothing changes) qualify, and only species that conditions actually set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgN92Vr4NPT21RjToXf6GS
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #579 +/- ##
==========================================
+ Coverage 79.48% 79.62% +0.14%
==========================================
Files 70 70
Lines 8278 8340 +62
Branches 1498 1515 +17
==========================================
+ Hits 6580 6641 +61
Misses 1207 1207
- Partials 491 492 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ExperimentsToSbmlConverterencodes every change made at the start of an experiment in the model. Each experiment gets an indicator parameter, and each changed target gets one initial assignment of the formPiecewise((v_1, ind_1 > 0.5), ..., (v_n, ind_n > 0.5), (default, True)). Since #546 that piecewise is flat, but it still has one branch per experiment.Froehlich_CellSystems2018, upgraded to v2, has 9169 experiments. Each has a single period that sets 145 targets: 122 parameters and 23 species, the gene expression levels, mutation status and initial protein amounts of a cell line. That is 1,329,505 changes, which become 9169 indicator parameters and 145 initial assignments of about 9169 branches each. With libpetab 0.10.0, pyPESTO's
PetabImporteron this problem was still in_create_initial_assignmentsafter 1 h 43 min and had not reached AMICI. On a reduced model whose targets are all parameters, AMICI's import then took more than 2 h on one core, and the generated code did not compile. Imported as PEtab v1, AMICI imports and generates code for the same model in 917 s.Change
A change now stays in the condition table if it sets a condition parameter: a per-experiment constant, like a column of a PEtab v1 condition table. A target qualifies if:
A parameter's changes are kept as they are. A species gets a parameter
_petab_initial_value_<id>that holds its initial value, and the initial assignment<id> = _petab_initial_value_<id>. Its changes become changes of that parameter.The kept changes go into the experiment's indicator condition (
_petab_experiment_condition_<id>), which both kept periods reference. So a value set in pre-equilibration persists into the main period, as it would in the model.An experiment now gets an indicator parameter only if one of its changes is encoded in the model, as an initial assignment branch or an event. For Froehlich, no indicators and no piecewise assignments remain: 23 initial assignments of the form above, and conversion takes 97 s.
AMICI needs no change. Its v2 importer already makes every remaining condition target a fixed parameter (
_do_import_sbml), andExperimentManager._set_constantssets those per experiment and period.The converter's output contract changes: a consumer now has to apply all changes of the kept periods, not just the indicators. The class docstring says so.
_simulateintests/v2/test_assignments_to_experiments.pysimulated the converted model by setting only the indicator; it now applies the kept conditions, as AMICI does. There is no opt-out flag; one could be added if a consumer needs everything encoded in the model.Verification
New tests in
tests/v2/test_converters.py:test_initial_assignments_many_experimentsnow also changesAin a later period, soAstill gets the flat piecewise that test is about.pytest tests/v2: 276 passed, 2 skipped (withbasicoinstalled).test_petablint_v2fails in my environment becausepetablintis not onPATH; it fails the same way onmain.ruff checkandruff formatare clean.AMICI on the benchmark collection (AMICI 1.2.0, nominal parameters). Each model was imported and simulated as v1, as v2 with libpetab 0.10.0, and as v2 with this branch; AMICI did all the evaluation. Problems with timepoint-specific overrides were flattened the same way for all three when AMICI required it (Fiedler).
SBMLException, and Armistead_CellDeathDis2024 segfaults during AMICI's v2 code generation.petab1to2and AMICI's v2 import, not from this change.A small problem through AMICI, with both libpetab versions. It covers condition parameters, a species set per experiment, a species changed alongside to an expression, pre-equilibration, and a parameter changed by an event. Log-likelihood −12299.399748 vs −12299.399747; outputs agree within solver tolerance. The patched model needs 3 indicators instead of 23.
Froehlich_CellSystems2018 through pyPESTO and AMICI, with this branch:
ExpDatas, which is a separate AMICI issue: PEtab v2 ExperimentManager: create_edatas is quadratic in the number of parameters (list membership in apply_parameters) AMICI-dev/AMICI#3519.🤖 Generated with Claude Code
https://claude.ai/code/session_01JgN92Vr4NPT21RjToXf6GS