Skip to content

fix(petab): read edition-2 parameter records on export, so a new-era free parameter no longer vanishes from the problem (#733) #735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wshlavacek merged 1 commit into from
Sep 17, 2026
Merged

fix(petab): read edition-2 parameter records on export, so a new-era free parameter no longer vanishes from the problem (#733) #735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,32 @@ All notable changes to PyBNF are documented below. This project adheres to
by default. Both surfaces are documented under gradient-based fitting.

### Fixed
- **The PEtab export reads edition-2 `parameter:` records, so a free parameter written the
new-era way no longer vanishes from the exported problem (#733).** The exporter picked its
free parameters out of the config by matching key names against `(_var$|^var$|^logvar$)`.
A `parameter:` record is stored under a `('parameter', id)` key, which matches none of the
three, so it was skipped rather than refused: no row was written, nothing downstream
noticed the id was missing, and the export finished cleanly having dropped the parameter.
A conf whose parameters were *all* records reported "No exportable free parameters found",
naming only the `*_var` keywords — pointing the user away from the syntax the edition-2
documentation teaches. Both contradict the exporter's own contract, that everything it
cannot write raises `NotImplementedError` with the boundary named in code.
The damage was widest on truncated priors. A record is the only grammar carrying
`lower`/`upper`, so it is what the importer emits for a prior truncated to a box
(ADR-0020/0047) — meaning a PEtab problem with bounded priors imported fine and then
exported to a table missing exactly those parameters. On the tutorial's own PEtab priors
problem (lesson 15) that was three of four: a log-normal, a gamma and a normal all
disappeared, leaving one plain uniform, and the re-import produced a fit over one
parameter instead of four.
The exporter now reads both declaration spellings in one pass, in declaration order, and
builds each record through the same mapping the fitter loads a job with, which moved to
`pybnf/parameter_record.py` so the two cannot drift. A record therefore meets the same
boundaries the positional line does, reached by a different spelling: a no-prior point
start, a natural-log sampling scale, a three-parameter family such as student_t, and the
log forms PEtab defines for no family are each refused with the keyword the record built.
`initial_value:` is honoured as the start point it is, alongside a `start_point` line when
the two agree and refused when they disagree, and an out-of-box one is a `PybnfError`
rather than the bare `OutOfBoundsException` that reaches users as "an unknown error".
- **The PEtab export writes the fit's start point, so a round trip no longer moves the fit
back to a sampled draw (#719).** `nominalValue` is where a PEtab problem states the point
a fit starts from, and since #583 PyBNF reads it: an imported problem's nominal becomes a
Expand Down
20 changes: 20 additions & 0 deletions ...-era-free-parameter-is-a-labeled-parameter-record-prior-bounds-initial-value.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,23 @@ prior — the default start when `initial_value` is absent), 0003 (prior in the
0031 (edition select-and-freeze), 0038 (which filed the native truncation grammar on #417, and the
blank-bounds export limitation this lifts). Issue: **#417** (reframed from "truncation grammar" to
"the new-era `parameter:` record"; truncation is its `lower`/`upper` fields).

## Amendment (2026-09-17, #733)

The loader is no longer config-private. It reads a declaration grammar, and the PEtab v2
exporter reads the same declarations when it serializes a job, so the mapping moved to
`pybnf/parameter_record.py` (`free_parameter_from_record`) with two callers:
`Configuration._load_variables` and `petab.export._free_parameters_from_conf`. The
`Configuration._free_parameter_from_record` method above remains, delegating, so the
references in this ADR and in ADR-0047 still name something real.

The move is what fixes #733. Until it, the exporter matched free parameters by config KEY
name (`_VAR_DECL`, which a `('parameter', id)` key never matches), so every `parameter:`
record was *skipped* rather than refused: the whole free parameter vanished from the
exported problem with no diagnostic, and with it every truncated prior — the shape this
ADR's `lower`/`upper` fields exist to author, and the one the importer emits as a record.
The failure is the same one #603 found on the coherence gate, in the other direction:
keying on the config key name instead of on what the declaration builds makes the record
syntax invisible to a rule the positional line goes through. An exporter that re-derived
the record grammar for itself would drift from the one the fitter runs, which is why the
fix is one shared builder rather than a second reading.
1 change: 1 addition & 0 deletions docs/modules/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ PyBNF Module References
measurement
noise
objective
parameter_record
parse
petab
printing
Expand Down
6 changes: 6 additions & 0 deletions docs/modules/parameter_record.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
=========================================================
Free-parameter records (:py:mod:`pybnf.parameter_record`)
=========================================================

.. automodule:: pybnf.parameter_record
:members:
6 changes: 5 additions & 1 deletion docs/petab.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ model) in ``out_dir``::
export_job('fit.conf', 'exported/')

The export is fit-preserving: re-importing the emitted problem reproduces the same
free parameters, priors, noise models, and data.
free parameters, priors, noise models, and data. Both free-parameter spellings are
read — the positional ``<family>_var`` line and the edition-2 ``parameter:`` record —
so a truncated prior, which only the record can state, exports and round-trips like any
other. Anything PEtab v2 cannot express raises ``NotImplementedError`` naming the
boundary; nothing is dropped quietly.

.. _petab_bngl_loader:

Expand Down
176 changes: 9 additions & 167 deletions pybnf/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from . import objective # noqa: F401 -- imported for its side effect: running the module fires the @register_objfunc decorators, populating OBJFUNC_REGISTRY before _load_obj_func dispatches.
from . import algorithms # noqa: F401 -- imported for its side effect: running the leaves fires the @register_fit_type decorators, populating FIT_TYPE_REGISTRY (incl. each method's config schema) before _build_config dispatches. No cycle: nothing in algorithms/ imports config.
from .registry import OBJFUNC_REGISTRY, FIT_TYPE_REGISTRY
from .priors import PRIOR_KEYWORD_MAP
from .parameter_record import free_parameter_from_record
from . import config_schema
from . import edition

Expand Down Expand Up @@ -3680,173 +3680,15 @@ def _warn_unbounded_start_points(self):
f"start point and the search bounded.")

def _free_parameter_from_record(self, pid, raw_fields, initialization_distribution):
"""Build a :class:`FreeParameter` from a new-era ``parameter:`` record (ADR-0043).

``raw_fields`` is the parsed ``{field: str}`` map -- every part of the line is named:
``prior`` (the family), ``space`` (``linear``/``log10``, the sampling-space transform),
the family's own distribution fields (``mean``/``sd``, ``location``/``scale``, ...),
``lower``/``upper`` (the bounds that truncate the prior -- #417/ADR-0020), and
``initial_value`` (the start point). No positional numbers; the family names its fields
via ``Prior.field_names``. The truncation/box capability is unchanged -- this only maps
named fields onto the existing ``FreeParameter`` constructor.
"""
fields = dict(raw_fields)

def _num(name):
v = fields.pop(name)
try:
return float(v)
except (TypeError, ValueError):
raise PybnfError(f"Parameter '{pid}': field '{name}' must be a number, got {v!r}.")

prior_name = fields.pop('prior', None)
# The sampling-space transform. PyBNF samples in linear, log10, or natural log; each
# base is named explicitly so it is never ambiguous (ADR-0022/0043). ``lin`` is
# accepted as PEtab's spelling of ``linear``; ``log`` is rejected as ambiguous (PEtab
# means natural by it, PyBNF historically means log10) -- write ``ln`` or ``log10``.
# The base prefixes the family keyword (``log{f}_var`` / ``ln{f}_var`` / ``var``).
pscale = str(fields.pop('parameter_scale', 'linear')).lower()
scale_prefix = {'lin': '', 'linear': '', 'log10': 'log', 'ln': 'ln'}
if pscale == 'log':
raise PybnfError(
f"Parameter '{pid}': parameter_scale 'log' is ambiguous -- write 'log10' "
f"(base 10) or 'ln' (natural log) explicitly (ADR-0022).")
if pscale not in scale_prefix:
raise PybnfError(f"Parameter '{pid}': parameter_scale must be 'linear', 'log10', or "
f"'ln', got '{pscale}'.")
prefix = scale_prefix[pscale]

lower = _num('lower') if 'lower' in fields else None
upper = _num('upper') if 'upper' in fields else None
# Bounds come as a pair: an open side is an explicit +-inf, never a blank
# (ADR-0047 -- no specification by absence). Omitting *both* is the untruncated
# shorthand. One-sided truncation IS supported now -- spell the open side with
# an infinity. The graded floor rule (positivity, support floor) is applied per
# path below: finite for a uniform box, the family floor for a truncated prior.
if (lower is None) != (upper is None):
present, absent = ('lower', 'upper') if upper is None else ('upper', 'lower')
raise PybnfError(
f"Parameter '{pid}': bounds come as a pair -- '{present}' is set but "
f"'{absent}' is missing. For an open {absent} side write an explicit "
f"infinity ('{absent}: inf' or '{absent}: -inf'), not a blank (ADR-0047).")
initial_value = _num('initial_value') if 'initial_value' in fields else None
is_log_scale = prefix in ('log', 'ln')

if prior_name is None:
if lower is not None:
# No prior but bounds -> uniform over the bounds (PEtab's default for an
# estimated parameter without an explicit prior; the importer does the same).
self._require_finite_box(pid, lower, upper, is_log_scale, "a uniform box")
keyword = f'{prefix}uniform_var'
self._reject_extra_fields(pid, fields, keyword)
return FreeParameter(pid, keyword, lower, upper, value=initial_value, bounded=True,
initialization_distribution=initialization_distribution)
# No prior and no bounds -> the no-prior start point (legacy var/logvar/lnvar). Its
# start value is carried in the FreeParameter's first slot *in sampling space*
# (Simplex reads it via from_sampling_space(p1)), so map the theta-space
# initial_value through the scale -- making initial_value the real value (theta) for
# a log start point too, consistent with the prior-param case.
if initial_value is None:
raise PybnfError(f"Parameter '{pid}': declares no prior, no bounds, and no "
f"initial_value -- nothing to fit. Give it a 'prior:', a "
f"'lower:'/'upper:' box, or an 'initial_value:'.")
if is_log_scale and initial_value <= 0.0:
raise PybnfError(f"Parameter '{pid}': a {pscale} start point needs "
f"initial_value > 0, got {initial_value}.")
self._reject_extra_fields(pid, fields, 'a no-prior start point')
_, start_scale = PRIOR_KEYWORD_MAP[f'{prefix}var']
return FreeParameter(pid, f'{prefix}var', float(start_scale.forward(initial_value)), None,
initialization_distribution=initialization_distribution)

prior_name = str(prior_name).lower()
if prior_name == 'uniform':
# Uniform: lower/upper ARE the support (and the bounds); no separate family fields.
if lower is None:
raise PybnfError(f"Parameter '{pid}': a uniform prior needs 'lower' and 'upper'.")
self._require_finite_box(pid, lower, upper, is_log_scale, "a uniform prior")
keyword = f'{prefix}uniform_var'
self._reject_extra_fields(pid, fields, keyword)
return FreeParameter(pid, keyword, lower, upper, value=initial_value, bounded=True,
initialization_distribution=initialization_distribution)

keyword = f'{prefix}{prior_name}_var'
if keyword not in PRIOR_KEYWORD_MAP:
raise PybnfError(f"Parameter '{pid}': unknown prior family '{prior_name}'.")
fam, _scale = PRIOR_KEYWORD_MAP[keyword]
params = []
for fname in fam.field_names:
if fname not in fields:
raise PybnfError(f"Parameter '{pid}': prior '{prior_name}' needs field '{fname}'.")
params.append(_num(fname))
self._reject_extra_fields(pid, fields, f"prior '{prior_name}'")
p1 = params[0]
p2 = params[1] if len(fam.field_names) >= 2 else None
# A three-parameter family (student_t, ADR-0057) carries its third value in p3;
# field_names ordered it last (df/location/scale -> p1/p2/p3). The carrier and
# build_prior pass it through; it is None for the one- and two-parameter families.
p3 = params[2] if len(fam.field_names) >= 3 else None
# lower/upper truncate an unbounded-support family to a reflecting box: two finite
# walls (two-sided, ADR-0020) or one finite wall + an infinity (half-bounded,
# ADR-0047). The graded floor rule warns/errors on a sub-floor lower bound first.
lower, upper = self._graded_truncation_bounds(pid, lower, upper, fam, _scale)
return FreeParameter(pid, keyword, p1, p2, lb=lower, ub=upper, value=initial_value,
initialization_distribution=initialization_distribution, p3=p3)

@staticmethod
def _require_finite_box(pid, lower, upper, is_log, where):
"""A Uniform family's bounds ARE its support, so they must be finite -- an
infinite bound describes an unbounded prior's open tail, not a box. A log
scale additionally needs a strictly positive lower bound (ADR-0047)."""
for label, v in (('lower', lower), ('upper', upper)):
if v is None or not np.isfinite(v):
raise PybnfError(
f"Parameter '{pid}': {where} needs a finite '{label}' bound "
f"(got {v}); an infinite bound describes an unbounded prior's open "
f"tail, not a uniform box.")
if is_log and lower <= 0.0:
raise PybnfError(
f"Parameter '{pid}': {where} on a log scale needs 'lower' > 0 "
f"(log of <= 0 is -inf), got lower={lower}.")

@staticmethod
def _graded_truncation_bounds(pid, lower, upper, fam, scale):
"""Apply the ADR-0047 graded sentinel/floor rule to a truncated family's bounds.

``lower``/``upper`` are in theta, already validated to be both-set or both-None
(the pairing rule). Omit-both passes through as the untruncated shorthand. On a
positive-support family -- whose theta floor, derived from the family's natural
support and the scale, is finite (0 for the linear half-bounded families;
0 for any log form; the doubly-unbounded families floor at -inf and are exempt) --
a sloppy-but-lossless ``lower: -inf`` is warned and canonicalized to the floor,
and a *finite* ``lower`` below the floor (a wall in the zero-density region, a
likely wrong family/scale) is an error. These families are all unbounded above,
so the upper side needs no floor. Returns the (possibly canonicalized) bounds."""
if lower is None:
return lower, upper
floor = scale.inverse(fam.support_lo_u) # theta-space support floor
if np.isfinite(floor):
if lower == -np.inf:
logger.warning(
f"Parameter '{pid}': 'lower: -inf' on a prior whose support floor "
f"is {floor:g} -- interpreting as open below at the floor. Write "
f"'lower: {floor:g}' to silence this (ADR-0047).")
lower = floor
elif lower < floor:
raise PybnfError(
f"Parameter '{pid}': 'lower: {lower:g}' is below the prior's support "
f"floor {floor:g} -- a finite wall in the zero-density region (likely "
f"a wrong family or scale). Use 'lower: {floor:g}' for an open lower "
f"side, or a value >= {floor:g} (ADR-0047).")
return lower, upper
"""Build a :class:`~pybnf.pset.FreeParameter` from a new-era ``parameter:`` record.

@staticmethod
def _reject_extra_fields(pid, leftover, where):
"""Raise a clear error if a ``parameter:`` record carries fields unknown to ``where``
(a typo or a field from a different family) -- naming every part means an unrecognised
name is an error, not a silently-ignored token."""
if leftover:
unknown = ', '.join(sorted(leftover))
raise PybnfError(f"Parameter '{pid}': unknown field(s) for {where}: {unknown}.")
The mapping itself lives in :mod:`pybnf.parameter_record`, because the PEtab
exporter reads the same records when it serializes a job and must land on the
same object this does -- it used to skip them entirely for want of a builder it
could reach (#733). Kept as a method so the loader below, and the ADR-0043 tests
that exercise the record grammar directly, still call one name.
"""
return free_parameter_from_record(pid, raw_fields, initialization_distribution)

@staticmethod
def _declaration_kind(v):
Expand Down
Loading
Loading