Repository navigation
v2: split and validate multi-model problems - #537
Open
FFroehlich wants to merge 3 commits into
Open
FFroehlich wants to merge 3 commits into
FFroehlich wants to merge 3 commits into
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.77% 77.22% +0.45%
==========================================
Files 67 67
Lines 7547 7597 +50
Branches 1346 1355 +9
==========================================
+ Hits 5794 5867 +73
+ Misses 1259 1240 -19
+ Partials 494 490 -4 ☔ 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.
Comment on lines
+2076
to
+2079
| full_problem_task_types = ( | ||
| CheckUnusedExperiments, | ||
| CheckUnusedConditions, | ||
| ) |
Comment on lines
+2183
to
+2188
| [ | ||
| m | ||
| for m in self.mappings | ||
| if m.model_id is None | ||
| or model.has_entity_with_id(m.model_id) | ||
| ] |
Comment on lines
+2102
to
+2106
| # 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]: |
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.
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;CheckUnusedExperimentsandCheckUnusedConditionson the full problem, because the split drops unused entries;Model `<id>`:;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