Repository navigation
fix(petab): undo petab 0.10.0's log(10) factor on a converted log10 observable's sigma - #1017
Merged
Merged
Conversation
…bservable's sigma petab 0.10.0 converts a v1 log10 observable to v2's natural-log family and writes its noise formula as log(10) * (<v1 formula>), which is exact for that family. petab1to2_preserve_scale states the scale differently (ADR-0073): it re-injects observableTransformation = log10 and resets noiseDistribution to the v1 linear base, and under that column the v1 sigma is the right one. Together the two gave a log10 residual with sigma ln 10 times the stated one. A numeric sigma imported as "sigma = formula log(10) * (0.1)" with no error (petab 0.9.0 imports "fix_at 0.1"); a placeholder sigma failed at import, which is the failure main's CI showed. restore_v1_noise_formulas puts each log10 observable's v1 noiseFormula back. It removes exactly the log(10) * (...) wrapper, around the v1 formula as petab's own v1 reader prints it, leaves an unchanged formula alone, and refuses any other rewrite with both formulas named. petab 0.10.0's new warning for this case joins the silenced patterns, since the converter repairs what it says. The output is now the same under petab 0.9 and 0.10. petab 0.10.0 also converts the v1 priors 0.9.x refused (logNormal, logLaplace, log10 parameterScaleUniform / parameterScaleLaplace). The converter already rewrites every declared prior from v1, so nothing changes there; on petab >= 0.10 those cases now run through the three existing prior oracles (hand-written v1 and v2 densities, PyBNF's imported prior, libpetab's own reading), and the refusal test runs only where petab still refuses. Tests: tests/test_petab_convert.py passes on petab 0.10.0 (156 passed, 8 skipped) and 0.9.0 (140 passed). With the restore disabled, both log10 full-conversion tests fail on 0.10.0. Signed-off-by: Bill Hlavacek <bill.hlavacek@gmail.com>
Independent review of the previous commit found a loud regression, reproduced by running it: on petab 0.9 with pandas < 3.1, a log10 observable whose numeric sigma has 17 significant digits was refused. The converter re-read the v2 observables table with pandas' default float parse, which moves about 40% of 17-digit values by an ulp, so the formula no longer matched the v1 text petab's own reader printed, and restore_v1_noise_formulas refused it with a message blaming petab1to2. The table is now read with dtype=str and keep_default_na=False, so nothing in it is reparsed; the hint no longer tells the user to switch petab release. Also from the review: the claim that the output is the same under petab 0.9 and 0.10 is narrowed to the noise formula (log10 + laplace, and some pre-equilibration shapes, still differ between the releases, loudly or as described below), and the test reads petab's version from package metadata, since petab.__version__ is deprecated. The reviewer's tests are added (tests/test_petab_convert_review.py). They check the objective PyBNF builds from a converted, imported problem against libpetab's own v1 likelihood (calculate_llh) across numeric, placeholder, parameter and prediction-dependent sigmas, several observables and two files, and the refusal of an unknown rewrite through the whole converter. One strict xfail remains, on petab 0.9 only: petab 0.9's petab1to2 drops a pre-equilibration whose condition changes nothing, with no warning. That is upstream and pre-existing, and is not addressed here. Tests: test_petab_convert.py + the review file pass on petab 0.10.0 / pandas 3.1.0 (173 passed, 8 skipped) and petab 0.9.0 / pandas 3.0.5 (156 passed, 1 xfailed). Signed-off-by: Bill Hlavacek <bill.hlavacek@gmail.com>
wshlavacek
added a commit
that referenced
this pull request
Nov 4, 2026
Bump __version__ 1.8.1 -> 1.8.2, with CITATION.cff and docs/conf.py, and fold the 108 staged fragments into CHANGELOG.md's [v1.8.2] section: 12 added, 10 changed, 1 removed, 85 fixed. Raise the bngsim floor from 0.16.0 to 0.17.0 (pyproject dependencies, the antimony extra, and the CI install in setup-pybnf), released together with this version. The fixes the test suite credits to bngsim main rather than to 0.16.0 are all in a released build from 0.17.0 on, so an install resolved from PyPI gets the answers those tests assert. The gradient fit's run-time gate still names 0.16.0, the oldest build that code is right on. The two packaging tests that pin the floor string follow it. Built on main after #1017 (petab 0.10) and #1021 (bngsim 0.17.0's per-species atol), which turned main's tests workflow green again. Checked against bngsim 0.17.0 and petab 0.10.0 from PyPI: test_packaging_metadata.py, test_sbml_solver_tolerances.py, test_gradient_sens_fallback.py and test_petab_convert.py, 387 passed, 9 skipped; uv build and twine check --strict pass, and the wheel declares Requires-Python >=3.12 and bngsim<1,>=0.17.0.
This was referenced Nov 4, 2026
Merged
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.
CI started installing petab 0.10.0 (released 2026-10-26), and 10 tests in
tests/test_petab_convert.pyfail on every leg. One of the causes is a silently wrong objective, which this PR fixes inpybnf/petab/convert.py.What petab 0.10.0 changed, and what it did to PyBNF
A v1
log10observable now converts with a rescaled noise formula. petab 0.10.0 writeslog10-normalaslog-normalwith noise formulalog(10) * (<v1 formula>). That is exact in v2's own terms.petab1to2_preserve_scalestates the scale differently (ADR-0073): it re-injectsobservableTransformation = log10and resetsnoiseDistributiontonormal. Under that column the v1 σ is the right one, so together the two gave a log10 residual whose σ was ln 10 times the stated one. For a v1 observable withnoiseFormula = 0.1:noise_model = lognormal, sigma = fix_at 0.1noise_model = lognormal, sigma = formula log(10) * (0.1), with no error or warningnoise_model = lognormal, sigma = fix_at 0.1A placeholder σ failed at import instead, which is the failure CI showed.
The priors petab 0.9.x refused now convert. These are
logNormal,logLaplace, andlog10parameterScaleUniform/parameterScaleLaplace. The converter already rewrites every declared prior from its v1 row (#893), so only the test that expected a refusal failed.The warning text changed. petab's warning for the log10 case is reworded, so it was no longer silenced.
Commits
fix(petab): undo petab 0.10.0's log(10) factor on a converted log10 observable's sigma.restore_v1_noise_formulasputs eachlog10observable's v1noiseFormulaback. It removes exactly thelog(10) * (...)wrapper and leaves an unchanged formula alone. Any other rewrite is refused as aPybnfErrornaming both formulas, since the cell sets σ.fix(petab): read the converted observables table as text. This fixes a review finding. On petab 0.9 with pandas < 3.1, a 17-digit numeric σ was refused, because the converter re-read the v2 table with a float parse that moves about 40% of such values by an ulp. The table is now read as text. Commit 2 also narrows my "same under 0.9 and 0.10" claim to the noise formula, and the test reads petab's version from package metadata.How it was checked
test_petab_convert.pyplus the review file give 173 passed, 8 skipped on petab 0.10.0 / pandas 3.1.0, and 156 passed, 1 xfailed on petab 0.9.0 / pandas 3.0.5.calculate_llh) on the original tables. The cases cover numeric, placeholder, fixed-parameter, expression and prediction-dependent σ; log10, log and lin; and several observables across two files.log(natural) observable: petab never rescales it.petab1to2, and the PEtab export doesn't call it.observables.tsvand the imported.confare byte-identical across releases.tests/test_petab_convert_review.py.Not in this PR (found in review)
petab1to2silently drops a pre-equilibration whose condition changes nothing. PyBNF then imports an experiment that starts from the initial conditions, which is a wrong objective, and pyproject still allowspetab>=0.9. petab 0.10 keeps it. This is upstream and pre-existing; a strict xfail on petab 0.9 records it. PyBNF could guard against it, or raise its floor to petab 0.10.