Repository navigation
v2: split and validate multi-model problems - #537
Conversation
Add `Problem.split_by_model`, which returns one single-model problem per model with the measurements, experiments, conditions, observables, mappings and parameters that model needs. `Problem.validate` now handles problems with more than one model: it checks the measurements' model IDs on the full problem, validates each part of the split and reports parameters that no model uses. Issues from a part are prefixed with its model ID. Also fix `Problem.__str__` for multi-model problems and add `model_id` to `Problem.add_measurement`. Closes #392. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #537 +/- ##
==========================================
+ Coverage 76.67% 77.16% +0.49%
==========================================
Files 67 67
Lines 7516 7568 +52
Branches 1339 1349 +10
==========================================
+ Hits 5763 5840 +77
+ Misses 1262 1242 -20
+ Partials 491 486 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Whole-table checks and extension mappings can be lost during splitting, producing incorrect validation results.
3 open findings
What changed in this PR
Adds multi-model splitting and validation support for PEtab v2 problems.
Changes:
- Adds
Problem.split_by_model()and model-aware validation. - Supports model IDs when adding measurements.
- Adds multi-model regression tests.
| File | Description |
|---|---|
petab/v2/core.py |
Implements splitting, validation, and measurement model IDs. |
tests/v2/test_core.py |
Tests multi-model splitting and validation. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| full_problem_task_types = ( | ||
| CheckUnusedExperiments, | ||
| CheckUnusedConditions, | ||
| ) |
There was a problem hiding this comment.
Fixed in 065ac9c: CheckUniquePrimaryKeys and CheckMappingTable now run on the full problem and are skipped in the per-model pass.
| [ | ||
| m | ||
| for m in self.mappings | ||
| if m.model_id is None | ||
| or model.has_entity_with_id(m.model_id) | ||
| ] |
There was a problem hiding this comment.
Fixed in 065ac9c: the split now drops only mappings that resolve in another model; mappings to non-model entities such as net1.parameters are kept in every part.
| # extraneous parameters are dropped by the split | ||
| used_parameter_ids = set().union( | ||
| *(sub_problem.x_ids for sub_problem in sub_problems.values()) | ||
| ) | ||
| if unused := [x for x in self.x_ids if x not in used_parameter_ids]: |
There was a problem hiding this comment.
Fixed in 065ac9c: the unused-parameter check only runs if CheckAllParametersPresentInParameterTable is among the requested tasks.
* Check unique IDs and the mapping table on the full problem, since the split drops unused entries and other models' mappings. * Keep mappings to non-model entities (e.g., SciML) in every part. * Only report unused parameters if parameter table checks are requested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes #392.
PEtab v2 problems can have more than one model. So far
Problem.validate()only warned about this, and every task that touchesProblem.modelfailed. As a result,validate().has_errors()was always true for such problems.Changes
New
Problem.split_by_model() -> dict[str, Problem]. It returns one single-model problem per model, containing:It raises on duplicate model IDs and on measurements that aren't assigned to a model. With a single model,
modelIdstays optional.Problem.validate()on multi-model problems:CheckMeasurementModelId, duplicate model IDs) on the full problem and stops there if they're invalid;CheckUniquePrimaryKeys,CheckMappingTable,CheckUnusedExperimentsandCheckUnusedConditionson the full problem, because the split drops unused entries and other models' mappings;Model `<id>`:;CheckAllParametersPresentInParameterTableis requested. The single-model extraneous-parameter check can't see them after the split.Problem.__str__no longer raises for multi-model problems.Problem.add_measurement(..., model_id=...).Downstream: pyPESTO ICB-DCM/pyPESTO#1772 adds multi-model PEtab v2 support. It currently carries its own copies of these helpers (
split_petab_problem_by_model,_validate_petab_v2_problem), which it can drop once this is released.🤖 Generated with Claude Code