Repository navigation
Conversation
dweindl
marked this pull request as ready for review
October 25, 2026 08:49
dweindl
requested review from
FFroehlich,
PaulJonasJost,
dilpath and
fbergmann
October 25, 2026 08:49
FFroehlich
approved these changes
Oct 25, 2026
dilpath
approved these changes
Oct 26, 2026
PaulJonasJost
approved these changes
Oct 26, 2026
dweindl
added a commit
to dweindl/libpetab-python
that referenced
this pull request
Oct 26, 2026
Placeholders that are overridden by reparametrized parameters are log-transformed, which requires all their overrides to be on log scale. `0` overrides were rejected so far, since `log(0) = -inf` is not a valid override value. They are now replaced by a new non-estimated parameter `negative_infinity` with nominal value `-inf` (assuming PEtab-dev/PEtab#687 is accepted). Only negative overrides still prevent the transformation. With `petab1to2(reparametrize="all")`, this reparametrizes all log-scaled parameters of Bachmann_MSB2011, so that the v1 and v2 log-priors agree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Allow `inf` and `-inf` as `nominalValue`. With the removal of `parameterScale`, fixing a log-transformed parameter at 0 requires a nominal value of `-inf`. Relates to #685. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
dweindl
force-pushed
the
v2_errata_inf_nominal
branch
from
October 26, 2026 13:26
434a55d to
efc534a
Compare
Member
|
As we allow Based on discussion at PEtab-dev/libpetab-python#539 (comment) |
dweindl
added a commit
to PEtab-dev/libpetab-python
that referenced
this pull request
Oct 26, 2026
PEtab v2 has no `parameterScale`. Converting a v1 `parameterScaleNormal(μ, σ)` prior on a `log10`-scaled parameter to `log-normal(μ ln10, σ ln10)` keeps the prior distribution, but adds a Jacobian term to the v2 MAP objective, which moves the optimum by `−σ² ln10` decades. No v2 prior over the unscaled parameter can preserve both the distribution and the optimum. `petab1to2` and `petab_files_1to2` get a `reparametrize` argument, which uses `petab.v2.reparametrize_log` (#560) to estimate `log10_<id>` (or `log_<id>`) instead of `<id>`: * `"none"` (default): current behaviour, now with a warning for converted `parameterScale*` priors * `"priors"`: log-scaled parameters with `parameterScale*` priors * `"all"`: all estimated log-scaled parameters without priors on linear scale (i.e., without `logNormal`, `uniform`, ...); this also preserves the v1 default prior (uniform on parameter scale) Log-scaled parameters sharing a placeholder with selected parameters are added. Such placeholders are substituted by `10^placeholder`, so numeric overrides are log-transformed. Since overrides must be finite, `0` overrides are replaced by a new fixed parameter `negative_infinity` with nominal value `-inf` (assuming PEtab-dev/PEtab#687 is accepted). With `"all"`, parameters without parameter-scale priors are skipped (with a warning) if their placeholder can't be log-transformed, i.e., if it is also overridden by negative numbers or by parameters that are not reparametrized. Parameters with `lowerBound = 0` (allowed in v1 for log-scaled parameters with an initialization prior) get `lowerBound = -inf` on log scale (PEtab-dev/PEtab#682). They are skipped with a warning if their prior on log scale would be an improper uniform prior, i.e., the v1 default prior if other estimated parameters have priors. Also fixes `petab1to2` adding default `uniform` priors to non-estimated parameters, and to all parameters if the v1 parameter table has an `objectivePriorType` column, but no estimated parameter has a prior. Priors of non-estimated parameters are converted as before. Non-estimated parameters are only reparametrized if they share a placeholder with selected parameters. A nominal value of `0` then becomes `-inf`, assuming PEtab-dev/PEtab#685 / PEtab-dev/PEtab#687 is accepted. Their `parameterScale*` priors are converted to log scale as well, unless they depend on missing bounds (then they are dropped with a warning). Checked on the benchmark collection: all 35 problems convert and pass v2 linting with `"all"` (now also tested in CI); with `"priors"`, 44 parameters are reparametrized in Bachmann, Isensee, Raimundez and Schwen. Checked with AMICI (v1 importer vs. v2 `PetabImporter`, forward sensitivities) on 30 benchmark problems converted with `"all"` (5 large ones excluded), at the nominal parameters and at a shifted point: log-likelihoods agree to a relative difference of ≤ 2e-8, and gradients agree to ≤ 2e-5 of the largest gradient entry, except for Crauste at the nominal parameters (4e-4 at rtol/atol 1e-12/1e-16, decreasing with tighter tolerances; 5e-14 at the shifted point). The remaining deviations are due to integration tolerances. Log-priors are identical for Bachmann, Isensee and Raimundez. (Before the `0` overrides were supported, 6 log-scaled Bachmann parameters couldn't be reparametrized, and their v1 default prior became a linear `uniform`, which preserves the MAP, but changes the log-prior by a constant.) The default is `"none"` for now, to keep `petab1to2` uninvasive, i.e., to not change the parametrization unless requested. Open to change. References Benchmarking-Initiative/Benchmark-Models-PEtab#333, PEtab-dev/PEtab#680, PEtab-dev/PEtab#683 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Extend erratum E3 to `observableParameters` and `noiseParameters` in the measurement table: numeric values there may be `inf` or `-inf`, as for `nominalValue`. Previously, an infinite value could only be given via a parameter from the parameter table, but not directly. Relates to #685. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
I agree. We should avoid the inconsistency of allowing overriding observable and noise parameter placeholders with +/- infinity via I updated this PR accordingly and re-requested reviews. |
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.
nominalValuewas specified asNUMERIC, i.e., finite. This conflicts with the statement that the removedparameterScalecan be replaced by transforming model parameters: a v1 parameter withparameterScale=logfixed atnominalValue=0corresponds to-inffor the log-transformed parameter.This adds erratum E3 (technical):
nominalValuemay beinfor-inf;NaNremains invalid. Infinite nominal values of non-estimated parameters are applied as specified. For estimated parameters,nominalValuedoes not affect the problem definition. The E1 entry now refers to E3 fornominalValue.The same applies to numeric values in
observableParametersandnoiseParametersin the measurement table, which could previously only be set to an infinite value via a parameter, but not directly. E3 covers those as well.Closes #685.
🤖 Generated with Claude Code