Skip to content

Commit 402393d

Browse files
FFroehlichclaude
andauthored
petab1to2: fix conversion of logNormal, logLaplace and parameterScaleUniform priors (#536)
Fixes three prior-conversion bugs in the PEtab v1 -> v2 converter (`update_prior` in `v1v2_parameter_df`). They turned up while verifying #535. ### Mappings | v1 `objectivePriorType` | `parameterScale` | v1 parameters | before | now | |---|---|---|---|---| | `logNormal` / `logLaplace` | any | `mu;sigma` | `logNormal` / `logLaplace` verbatim → pydantic `ValidationError` when building `v2.Parameter` | `log-normal` / `log-laplace`, `mu;sigma` | | `parameterScaleUniform` | `log` | `a;b` (ln-space) | `log-uniform` with `a;b` → prior pdf is NaN | `log-uniform` with `exp(a);exp(b)`, clipped to the bounds | | `parameterScaleUniform` | `log10` | `a;b` (log10-space) | `NotImplementedError` (`log10-uniform`) | `log-uniform` with `10^a;10^b`, clipped to the bounds | Why these are correct: 1. **logNormal / logLaplace.** In v1 these are always based on the natural logarithm, whatever the `parameterScale` (`Prior` maps `(LOG_NORMAL, _)` to `Normal(..., log=True)`). That is the same convention as v2 `log-normal` / `log-laplace`, so only the name changes. 2. **and 3. parameterScaleUniform on log/log10.** If ln(x) (or log10(x)) is uniform on [a, b], then x is log-uniform on [e^a, e^b] (or [10^a, 10^b]). v2 `log-uniform` takes the bounds of x itself as parameters. `Parameter.prior_dist` notes this ("Mind the different interpretation of distribution parameters for `Uniform(..., log=True)` and `LogUniform`"). So the parameters are transformed back to linear scale. Like v1's `Prior`, the scaled interval [a, b] is first clipped to the scaled parameter bounds, and the linear bounds are used for clipped endpoints. Within the bounds this gives the same truncated density, and it avoids overflow for valid but wide intervals such as log10 `-400;400`. Missing `objectivePriorParameters` for `parameterScaleUniform` stay missing, so `update_prior_pars` still fills in `lowerBound;upperBound`. In v1 they default to the scaled bounds, which is the same distribution. Not changed: a missing `objectivePriorType` (v1 default `parameterScaleUniform`) still converts to `uniform` with `lowerBound;upperBound` on any scale. On log-scaled parameters the density differs from v1's log-uniform default. But this matches the v2 spec's default for parameters without a prior, and it keeps the optimum of v1 problems that have no priors. ### Verification For each case I compared `petab.v1.priors.Prior.from_par_dict(row, type_="objective").pdf(x)` with the converted `v2.Parameter.prior_dist.pdf(x)` on a grid spanning the bounds [1e-3, 1e2]. This included prior ranges inside the bounds, wider than the bounds (truncation) and partly outside them. All agree to rel. 1e-12. Tests: `test_petab1to2_parameter_scale_priors` (from #535) is now `test_petab1to2_priors`. It is parametrized over prior parameters and adds the cases above: log/log10 `parameterScaleUniform` within bounds, exceeding bounds (including `-400;400` / `-1000;1000`, which overflow when unscaled), and with missing parameters, plus `logNormal`/`logLaplace` on `lin` and `log10` scale. 11 of the new cases fail on the previous converter code, and all pass with this change. `pytest tests/v2` passes (141 passed), including `test_benchmark_collection` with `benchmark_models_petab` installed. `pre-commit` (ruff, ruff-format) is clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 37b7025 commit 402393d

2 files changed

Lines changed: 59 additions & 12 deletions

File tree

‎petab/v2/petab1to2.py‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from pandas.io.common import get_handle, is_url
1616

1717
from .. import v1, v2
18+
from ..v1.parameters import scale, unscale
1819
from ..v1.yaml import get_path_prefix, load_yaml
1920
from ..v1.yaml import validate as validate_yaml
2021
from ..versions import get_major_version
@@ -516,7 +517,36 @@ def update_prior(row) -> tuple[str, str | None]:
516517
pscale = v1.C.LIN
517518

518519
if prior_type not in v1.C.PARAMETER_SCALE_PRIOR_TYPES:
519-
return prior_type, prior_pars
520+
# v1 logNormal / logLaplace are based on the natural logarithm,
521+
# just like v2 log-normal / log-laplace
522+
return {
523+
v1.C.LOG_NORMAL: v2.C.LOG_NORMAL,
524+
v1.C.LOG_LAPLACE: v2.C.LOG_LAPLACE,
525+
}.get(prior_type, prior_type), prior_pars
526+
527+
if prior_type == v1.C.PARAMETER_SCALE_UNIFORM and pscale != v1.C.LIN:
528+
# A uniform distribution of ln(x) (log10(x)) on [a, b] is a
529+
# log-uniform distribution of x on [e^a, e^b] ([10^a, 10^b]).
530+
# v2 log-uniform takes the bounds of x as parameters.
531+
# Missing parameters default to the parameter bounds (see
532+
# `update_prior_pars` below), as in v1.
533+
if not pd.isna(prior_pars):
534+
# Clip [a, b] to the bounds before unscaling, as v1 does
535+
# (see `petab.v1.priors.Prior`). This avoids overflow for
536+
# intervals that exceed the floating-point range when
537+
# unscaled.
538+
low, high = map(
539+
float, str(prior_pars).split(v1.C.PARAMETER_SEPARATOR)
540+
)
541+
lb, ub = row[v1.C.LOWER_BOUND], row[v1.C.UPPER_BOUND]
542+
low = lb if low <= scale(lb, pscale) else unscale(low, pscale)
543+
high = (
544+
ub if high >= scale(ub, pscale) else unscale(high, pscale)
545+
)
546+
prior_pars = (
547+
f"{float(low)}{v2.C.PARAMETER_SEPARATOR}{float(high)}"
548+
)
549+
return v2.C.LOG_UNIFORM, prior_pars
520550

521551
if pscale == v1.C.LOG10 and prior_type in (
522552
v1.C.PARAMETER_SCALE_NORMAL,

‎tests/v2/test_conversion.py‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -197,24 +197,41 @@ def simulate(measurement_df):
197197
"ignore:.*Parameter scales are not supported in PEtab v2.*:UserWarning"
198198
)
199199
@pytest.mark.parametrize(
200-
"parameter_scale, prior_type",
200+
"parameter_scale, prior_type, prior_parameters",
201201
[
202-
(v1.C.LOG10, v1.C.PARAMETER_SCALE_NORMAL),
203-
(v1.C.LOG10, v1.C.PARAMETER_SCALE_LAPLACE),
204-
(v1.C.LOG, v1.C.PARAMETER_SCALE_NORMAL),
205-
(v1.C.LOG, v1.C.PARAMETER_SCALE_LAPLACE),
206-
(v1.C.LIN, v1.C.PARAMETER_SCALE_NORMAL),
202+
(v1.C.LOG10, v1.C.PARAMETER_SCALE_NORMAL, "-0.5;0.7"),
203+
(v1.C.LOG10, v1.C.PARAMETER_SCALE_LAPLACE, "-0.5;0.7"),
204+
(v1.C.LOG, v1.C.PARAMETER_SCALE_NORMAL, "-0.5;0.7"),
205+
(v1.C.LOG, v1.C.PARAMETER_SCALE_LAPLACE, "-0.5;0.7"),
206+
(v1.C.LIN, v1.C.PARAMETER_SCALE_NORMAL, "-0.5;0.7"),
207+
# parameterScaleUniform within the bounds, exceeding the bounds
208+
# (also beyond the floating-point range when unscaled),
209+
# and defaulting to the bounds
210+
(v1.C.LOG10, v1.C.PARAMETER_SCALE_UNIFORM, "-0.5;0.7"),
211+
(v1.C.LOG10, v1.C.PARAMETER_SCALE_UNIFORM, "-5;0.7"),
212+
(v1.C.LOG10, v1.C.PARAMETER_SCALE_UNIFORM, "-400;400"),
213+
(v1.C.LOG10, v1.C.PARAMETER_SCALE_UNIFORM, np.nan),
214+
(v1.C.LOG, v1.C.PARAMETER_SCALE_UNIFORM, "-0.5;0.7"),
215+
(v1.C.LOG, v1.C.PARAMETER_SCALE_UNIFORM, "-10;0.7"),
216+
(v1.C.LOG, v1.C.PARAMETER_SCALE_UNIFORM, "-1000;1000"),
217+
(v1.C.LOG, v1.C.PARAMETER_SCALE_UNIFORM, np.nan),
218+
(v1.C.LIN, v1.C.PARAMETER_SCALE_UNIFORM, "0.1;0.7"),
219+
# logNormal / logLaplace do not depend on the parameter scale
220+
(v1.C.LOG10, v1.C.LOG_NORMAL, "-0.5;0.7"),
221+
(v1.C.LOG10, v1.C.LOG_LAPLACE, "-0.5;0.7"),
222+
(v1.C.LIN, v1.C.LOG_NORMAL, "-0.5;0.7"),
223+
(v1.C.LIN, v1.C.LOG_LAPLACE, "-0.5;0.7"),
207224
],
208225
)
209-
def test_petab1to2_parameter_scale_priors(
210-
tmp_path, parameter_scale, prior_type
226+
def test_petab1to2_priors(
227+
tmp_path, parameter_scale, prior_type, prior_parameters
211228
):
212-
"""Test that parameterScale{Normal,Laplace} priors are converted to
213-
priors with the same density."""
229+
"""Test that objective priors are converted to priors with the same
230+
density."""
214231
k1 = _K1 | {
215232
v1.C.PARAMETER_SCALE: parameter_scale,
216233
v1.C.OBJECTIVE_PRIOR_TYPE: prior_type,
217-
v1.C.OBJECTIVE_PRIOR_PARAMETERS: "-0.5;0.7",
234+
v1.C.OBJECTIVE_PRIOR_PARAMETERS: prior_parameters,
218235
}
219236
yaml_file = _write_v1_problem(
220237
tmp_path,

0 commit comments

Comments
 (0)