diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index e1f935b2..e5a511ce 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -25,3 +25,4 @@ - [ ] The PEtab problem author(s) are assigned to the GitHub issue - [ ] The README has been updated with `bmp-create-overview --update` (requires `pip install -e src/python` from the repository root) - [ ] The new PEtab problem row in the generated table has the correct reference (and other entries) +- [ ] If this PR results in a problem having both `v1/` and `v2/` encodings, I've confirmed they satisfy the v1/v2 equivalence policy in [CONTRIBUTING.md](../CONTRIBUTING.md#v1v2-equivalence) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cf4c7bbc..def5998e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,10 +2,11 @@ New submissions are very welcome! Please check our [pull request template](.gith Simply create a new branch and open a pull request with your files. We will then check your model and merge it into the collection. -Each problem lives under `problems//`: -- `v1/` contains the PEtab problem itself: `problem.yaml`, `model.xml`, `conditions.tsv`, - `measurements.tsv`, `observables.tsv`, `parameters.tsv`, and optionally `simulations.tsv` - and `visualizations.tsv`. +Each problem lives under `problems//`, with at least one of `v1/`/`v2/` present: +- `v1/` holds the problem in PEtab v1 format: `problem.yaml`, `model.xml`, + `conditions.tsv`, `measurements.tsv`, `observables.tsv`, `parameters.tsv`, and optionally + `simulations.tsv` and `visualizations.tsv`. +- `v2/` holds the same problem in PEtab v2 format. - `metadata.yaml` contains version-independent metadata. Currently, this is a single-sentence `short_description` of the problem, stating the type of model (e.g., biochemical reaction network, compartmental epidemiological model), the biological system, and, where informative, the data. @@ -20,3 +21,19 @@ Each problem lives under `problems//`: - `resources/` (optional) holds anything else related to the problem that isn't part of the PEtab files themselves (e.g. raw data, scripts, notebooks, figures). There's no prescribed structure — organize it however fits your problem. + +## v1/v2 equivalence + +A problem's `v1/` and `v2/` encodings must represent *exactly* the same mathematical problem: +same model, same data, same objective. + +This is a constraint on the math, not the encoding: `v2/` files may (and often should) look +structurally different from a mechanical v1-to-v2 port to make idiomatic use of v2 features +(experiments table, v2 prior/distribution syntax, mapping tables, ...) where they fit better. +They just have to evaluate to the same log-likelihood / log-posterior / gradient at nominal +parameters as `v1/`. + +For problems with priors (maximum a posteriori estimation), the policy is not decided yet: +v1 priors on log-scaled parameters cannot, in general, be expressed as v2 priors without +changing either the optimum or the prior distribution. See +[#333](https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/issues/333). diff --git a/src/python/benchmark_models_petab/C.py b/src/python/benchmark_models_petab/C.py index 46b52a97..e4e54aec 100644 --- a/src/python/benchmark_models_petab/C.py +++ b/src/python/benchmark_models_petab/C.py @@ -12,6 +12,7 @@ # layout of a single problem directory (`//...`) V1_DIRNAME: str = "v1" +V2_DIRNAME: str = "v2" PROBLEM_FILENAME: str = "problem.yaml" SIMULATIONS_FILENAME: str = "simulations.tsv" METADATA_FILENAME: str = "metadata.yaml" diff --git a/src/python/benchmark_models_petab/__init__.py b/src/python/benchmark_models_petab/__init__.py index 4856ab34..c901db29 100644 --- a/src/python/benchmark_models_petab/__init__.py +++ b/src/python/benchmark_models_petab/__init__.py @@ -4,7 +4,8 @@ Python tool to access the model collection. """ -from .base import get_problem, get_problem_yaml_path, get_simulation_df +from . import v1, v2 +from .v1 import get_problem, get_problem_yaml_path, get_simulation_df from .C import MODEL_DIRS, MODELS, MODELS_DIR from .metadata import ProblemMetadata, get_metadata from .overview import get_overview_df diff --git a/src/python/benchmark_models_petab/base.py b/src/python/benchmark_models_petab/v1.py similarity index 90% rename from src/python/benchmark_models_petab/base.py rename to src/python/benchmark_models_petab/v1.py index 832b0a7e..b3f66794 100644 --- a/src/python/benchmark_models_petab/base.py +++ b/src/python/benchmark_models_petab/v1.py @@ -1,4 +1,4 @@ -"""Get a petab problem from the collection.""" +"""Get a PEtab v1 problem from the collection.""" from pathlib import Path @@ -22,7 +22,9 @@ def get_problem_yaml_path(id_: str) -> Path: """ yaml_path = Path(MODELS_DIR, id_, V1_DIRNAME, PROBLEM_FILENAME) if not yaml_path.exists(): - raise ValueError(f"Could not find YAML for problem with ID `{id_}`.") + raise ValueError( + f"Could not find a v1 YAML for problem with ID `{id_}`." + ) return yaml_path diff --git a/src/python/benchmark_models_petab/v2.py b/src/python/benchmark_models_petab/v2.py new file mode 100644 index 00000000..3a459eff --- /dev/null +++ b/src/python/benchmark_models_petab/v2.py @@ -0,0 +1,63 @@ +"""Get a PEtab v2 problem from the collection.""" + +from pathlib import Path + +import pandas as pd +import petab.v2 as petab + +from .C import MODELS_DIR, PROBLEM_FILENAME, SIMULATIONS_FILENAME, V2_DIRNAME + + +def get_problem_yaml_path(id_: str) -> Path: + """Get the path to the PEtab v2 problem YAML file. + + Parameters + ---------- + id_: Problem name, as in `benchmark_models_petab.MODELS`. + + Returns + ------- + The path to the PEtab v2 problem YAML file. + """ + yaml_path = Path(MODELS_DIR, id_, V2_DIRNAME, PROBLEM_FILENAME) + if not yaml_path.exists(): + raise ValueError( + f"Could not find a v2 YAML for problem with ID `{id_}`." + ) + return yaml_path + + +def get_problem(id_: str) -> petab.Problem: + """Read the PEtab v2 problem from the benchmark collection by name. + + Parameters + ---------- + id_: Problem name, as in `benchmark_models_petab.MODELS`. + + Returns + ------- + The PEtab v2 problem. + """ + yaml_file = get_problem_yaml_path(id_) + return petab.Problem.from_yaml(yaml_file) + + +def get_simulation_df(id_: str) -> pd.DataFrame | None: + """Get the simulation dataframe for the v2 encoding of the benchmark + collection problem with the given name. + + Parameters + ---------- + id_: Problem name, as in `benchmark_models_petab.MODELS`. + + Returns + ------- + The simulation dataframe if it exists, else None. + """ + path = Path(MODELS_DIR, id_, V2_DIRNAME, SIMULATIONS_FILENAME) + if path.is_file(): + return pd.read_csv( + path, sep="\t", index_col=None, float_precision="round_trip" + ) + + return None diff --git a/src/python/test/test_base.py b/src/python/test/test_v1.py similarity index 65% rename from src/python/test/test_base.py rename to src/python/test/test_v1.py index 282a4f6e..42aedfed 100644 --- a/src/python/test/test_base.py +++ b/src/python/test/test_v1.py @@ -19,3 +19,10 @@ def test_get_problem(): def test_get_simulation_df(): assert models.get_simulation_df("Elowitz_Nature2000").empty is False assert models.get_simulation_df("not a problem name") is None + + +def test_v1_submodule_matches_top_level(): + """The top-level functions are the v1 ones, unchanged.""" + assert models.get_problem is models.v1.get_problem + assert models.get_problem_yaml_path is models.v1.get_problem_yaml_path + assert models.get_simulation_df is models.v1.get_simulation_df diff --git a/src/python/test/test_v2.py b/src/python/test/test_v2.py new file mode 100644 index 00000000..ced61479 --- /dev/null +++ b/src/python/test/test_v2.py @@ -0,0 +1,25 @@ +"""Tests for the v2 accessors. + +No problem in the collection has a `v2/` encoding yet, so only the +"not found" error path can be tested here. Once real `problems//v2/` +directories exist, add a happy-path test (`get_problem`/`get_simulation_df` +on an actual v2 problem) alongside these. +""" + +import pytest + +import benchmark_models_petab as models + + +def test_get_problem_yaml_path_raises_for_missing_v2(): + with pytest.raises(ValueError, match="v2"): + models.v2.get_problem_yaml_path(models.MODELS[0]) + + +def test_get_problem_raises_for_missing_v2(): + with pytest.raises(ValueError, match="v2"): + models.v2.get_problem(models.MODELS[0]) + + +def test_get_simulation_df_returns_none_for_missing_v2(): + assert models.v2.get_simulation_df(models.MODELS[0]) is None