fix(petab): reset a re-injected log observable's noiseDistribution to its v1 base family (#679)

docs/adr/0073-petab-v1-log10-observabletransformation-is-re-injected-as-a-preserved-column-and-imported-as-the-native-lognormal-family.md

@@ -118,3 +118,35 @@ log10, PEtab `log-normal` is natural log"), **0032** (the importer read path tha
 `noiseDistribution`), **0011** (the per-point kernel and the median location). Sibling:
 **#491** (`petab1to2_preserve_scale` re-injecting `parameterScale` as a `log-uniform` prior —
 the parameter-axis twin). Related: the benchmark build siblings (#492–#496).
+
+## Addendum (2026-09-10): the converter also resets `noiseDistribution` to the v1 base family (issue #679)
+
+**Accepted and implemented 2026-09-10.** The re-injection above was silently leaning on a
+petab bug. `petab1to2` is *designed* to fold the v1 `observableTransformation` into the v2
+`noiseDistribution`; a missing `return` in that merge left the column blank in every
+petab < 0.9.0, the importer defaulted the blank to `normal`, and `log10` over `normal`
+imported as `lognormal` exactly as this ADR describes. petab 0.9.0 (2026-09-07,
+PEtab-dev/libpetab-python#502) fixed the merge. Because PEtab v2 has no `log10-normal`, the
+converter now substitutes the natural-log family — `log10` + `normal` → `log-normal`, with a
+warning — which is a silent `ln 10` rescaling of sigma. Stacked under our re-injected `log10`
+column, the importer's "give the residual scale in one place" rule refused every converted
+log10 observable as a contradiction, and `test_converted_log10_problem_imports_as_lognormal`
+went red on every Python ≥ 3.12 CI leg (petab 0.9.0 requires 3.12, so 3.11 stayed on 0.8.2
+and green).
+
+The fix keeps the ADR's design and closes the gap it left: `petab1to2_preserve_scale` now
+reads each log observable's **v1** `noiseDistribution` (blank → `normal`) alongside its
+transformation, and `inject_observable_transformations` takes an optional
+`{observableId: 'normal' | 'laplace'}` map and resets the row's `noiseDistribution` to that
+linear base while writing the transformation. The scale is then stated once, in the preserved
+column, regardless of which petab produced the v2 table — a blank cell (0.8.2) and a folded
+`log-normal` / `log-laplace` (0.9.0) both come back to the v1 family. Rows without a
+transformation keep whatever petab1to2 wrote, so a linear problem is still byte-identical.
+petab's own substitution warning is inside the `catch_warnings` block the converter already
+uses to silence the `parameterScale` warning, for the same reason: re-adding the scale is
+this function's job.
+
+This is not an upstream bug. The substitution is documented and warned, and the underlying
+gap — no log10-normal in PEtab v2 — is a specification decision (ADR-0022's "PyBNF
+`lognormal` is log10, PEtab `log-normal` is natural log"). The preserved column is the
+workaround the specification leaves us.

pybnf/petab/convert.py

@@ -27,15 +27,20 @@
 change-of-variables Jacobian) -- a *different objective* from the linear residual, not just
 a different search. PEtab v2 removed the column and folded transformation into
 ``noiseDistribution`` as **natural-log** ``log-normal`` / ``log-laplace`` prefixes, with **no
-log10 form**; ``petab1to2`` therefore drops a v1 ``log10`` transformation entirely (it
-downgrades ``log10-normal`` to a blank ``noiseDistribution``), so the observable imports as a
-linear Gaussian and the fit optimises the wrong objective. Since v2 has no faithful
+log10 form**; ``petab1to2`` therefore cannot carry a v1 ``log10`` transformation faithfully.
+petab < 0.9.0 dropped it entirely (a missing ``return`` left ``noiseDistribution`` blank, so
+the observable imported as a linear Gaussian and the fit optimised the wrong objective);
+petab >= 0.9.0 substitutes the natural-log family (``log10-normal`` -> ``log-normal``, with a
+warning), which silently rescales sigma by ``ln 10``. Since v2 has no faithful
 representation, we re-inject ``observableTransformation`` as a **preserved extra column** on
-the v2 observables table: PyBNF's importer reads it to select the noise family's additive
-scale (``lin`` / ``log10`` / ``log``; :mod:`pybnf.petab.observables`,
-:mod:`pybnf.petab.import_`), and other PEtab v2 tools ignore the unknown column (it passes v2
-lint). This is directly parallel to the parameterScale re-injection above -- the same
-"re-add the scale petab1to2 dropped" migration, on the observable axis.
+the v2 observables table and reset ``noiseDistribution`` to the v1 **linear base family**
+(``normal`` / ``laplace``), so the residual scale is stated in exactly one place: PyBNF's
+importer reads the column to select the noise family's additive scale (``lin`` / ``log10`` /
+``log``; :mod:`pybnf.petab.observables`, :mod:`pybnf.petab.import_`) and refuses a log
+transformation over an already-log distribution as a contradiction (issue #679), while other
+PEtab v2 tools ignore the unknown column (it passes v2 lint). This is directly parallel to
+the parameterScale re-injection above -- the same "re-add the scale petab1to2 dropped"
+migration, on the observable axis.
 
 This is the migration ``petab1to2`` should offer as an opt-in; it lives here as an
 explicit, named converter so :func:`pybnf.petab.import_job` stays a pure v2 importer with
@@ -61,6 +66,11 @@
 #: v1 spelling; PyBNF's importer reads it, other v2 tools ignore it).
 _OBSERVABLE_TRANSFORMATION_COLUMN = 'observableTransformation'
 
+#: The v2 observables column petab1to2 folds the v1 transformation into (as the natural-log
+#: family, petab >= 0.9.0; blank before). Reset to the v1 linear base family alongside the
+#: re-injected transformation so the scale is stated once (issue #679).
+_NOISE_DISTRIBUTION_COLUMN = 'noiseDistribution'
+
 
 def petab1to2_preserve_scale(v1_yaml_path, out_dir):
     """Convert a PEtab **v1** problem to **v2**, preserving log estimation scales.
@@ -123,12 +133,18 @@ def petab1to2_preserve_scale(v1_yaml_path, out_dir):
     # 4. Re-inject the dropped observableTransformation as a preserved column (issue #499).
     #    v2 has no log10 noiseDistribution, so a log/log10 observable has no v2-native home;
     #    the importer reads this extra column to pick the noise family's additive scale.
-    transformations = _v1_observable_transformations(v1_yaml_path, v1_spec)
-    if transformations:
+    #    petab1to2 folded the same transformation into noiseDistribution as the natural-log
+    #    family (petab >= 0.9.0; blank before), so that column is reset to the v1 linear base
+    #    (normal / laplace) -- the importer refuses a log transformation over an already-log
+    #    distribution as a contradiction (issue #679).
+    scales = _v1_observable_scales(v1_yaml_path, v1_spec)
+    if scales:
+        transformations = {oid: transformation for oid, (transformation, _) in scales.items()}
+        distributions = {oid: distribution for oid, (_, distribution) in scales.items()}
         for obs_file in v2_spec.get('observable_files', []):
             v2_obs_path = out_dir / obs_file
             v2_odf = pd.read_csv(v2_obs_path, sep='\t')
-            inject_observable_transformations(v2_odf, transformations)
+            inject_observable_transformations(v2_odf, transformations, distributions)
             v2_odf.to_csv(v2_obs_path, sep='\t', index=False)
 
     return v2_yaml
@@ -185,7 +201,7 @@ def inject_log_uniform_priors(v2_pdf, log_estimated_ids, declared_prior_ids=None
     return v2_pdf
 
 
-def inject_observable_transformations(v2_odf, transformations):
+def inject_observable_transformations(v2_odf, transformations, distributions=None):
     """Add an ``observableTransformation`` column to a v2 observables DataFrame in place.
 
     ``transformations`` is a ``{observableId: 'log' | 'log10'}`` map (linear observables are
@@ -194,32 +210,54 @@ def inject_observable_transformations(v2_odf, transformations):
     PEtab v2 dropped the column, so this is a **preserved extra column** the importer reads to
     select the noise family's additive scale (:mod:`pybnf.petab.observables`); other v2 tools
     ignore it. Mutates and returns ``v2_odf`` (a v2 observables :class:`pandas.DataFrame`).
+
+    ``distributions``, when given, is the matching ``{observableId: 'normal' | 'laplace'}``
+    map of each log observable's **v1** ``noiseDistribution`` -- its linear base family. Every
+    row that receives a transformation also has its ``noiseDistribution`` reset to that base.
+    petab1to2 folds the v1 transformation into this column as the natural-log family
+    (``log10-normal`` -> ``log-normal``, petab >= 0.9.0; a blank cell before that), and the
+    importer refuses a log transformation stacked over an already-log distribution, so the
+    scale must be stated in the transformation column alone (issue #679). Rows not in the
+    map keep whatever distribution petab1to2 wrote.
     """
     col = _OBSERVABLE_TRANSFORMATION_COLUMN
     if col not in v2_odf.columns:
         v2_odf[col] = ''
     # petab1to2 may emit an all-empty column as float64 (NaN); coerce to object so the string
     # cells below don't raise a dtype error. Untouched rows still write as blank.
     v2_odf[col] = v2_odf[col].astype('object')
+    dcol = _NOISE_DISTRIBUTION_COLUMN
+    if distributions:
+        if dcol not in v2_odf.columns:
+            v2_odf[dcol] = ''
+        v2_odf[dcol] = v2_odf[dcol].astype('object')
     for i, row in v2_odf.iterrows():
-        transformation = transformations.get(str(row['observableId']))
+        oid = str(row['observableId'])
+        transformation = transformations.get(oid)
         if transformation is not None:
             v2_odf.at[i, col] = transformation
+            if distributions and oid in distributions:
+                v2_odf.at[i, dcol] = distributions[oid]
     return v2_odf
 
 
-def _v1_observable_transformations(v1_yaml_path, v1_spec):
-    """``{observableId: 'log' | 'log10'}`` for every v1 observable with a log residual scale.
+def _v1_observable_scales(v1_yaml_path, v1_spec):
+    """``{observableId: ('log' | 'log10', 'normal' | 'laplace')}`` for every v1 observable
+    with a log residual scale: its transformation and the linear base family of its v1
+    ``noiseDistribution`` (blank -> ``normal``, the v1 default).
 
     Reads each v1 problem's observable file(s) and keeps only the ``log`` / ``log10``
     transformations (linear -- or an absent column -- is the v2 default and needs no
-    re-injection). ``petab.v2.petab1to2`` drops this column, so the scale-preserving converter
-    re-injects what this returns (issue #499), mirroring the parameterScale re-injection.
+    re-injection). ``petab.v2.petab1to2`` drops the transformation column and folds it into
+    ``noiseDistribution``, so the scale-preserving converter re-injects the transformation and
+    restores the base distribution from what this returns (issues #499, #679), mirroring the
+    parameterScale re-injection.
     """
+    import pandas as pd
     import petab.v1 as petab_v1
     import petab.v1.C as C1
 
-    transformations = {}
+    scales = {}
     for problem in v1_spec.get('problems', []):
         for obs_file in problem.get('observable_files', []):
             odf = petab_v1.get_observable_df(str(Path(v1_yaml_path).parent / obs_file))
@@ -228,9 +266,13 @@ def _v1_observable_transformations(v1_yaml_path, v1_spec):
             for oid, row in odf.iterrows():
                 value = row.get(C1.OBSERVABLE_TRANSFORMATION)
                 transformation = str(value if value is not None else C1.LIN).strip().lower()
-                if transformation in _LOG_TRANSFORMATIONS:
-                    transformations[str(oid)] = transformation
-    return transformations
+                if transformation not in _LOG_TRANSFORMATIONS:
+                    continue
+                dist = row.get(C1.NOISE_DISTRIBUTION)
+                if dist is None or pd.isna(dist) or not str(dist).strip():
+                    dist = C1.NORMAL
+                scales[str(oid)] = (transformation, str(dist).strip().lower())
+    return scales
 
 
 def _sole_yaml(out_dir):

tests/test_petab_convert.py

@@ -80,6 +80,18 @@ def test_log10_observable_transformation_is_reinjected(self, tmp_path):
         row = obs.set_index('observableId').loc['obs_V']
         assert row['observableTransformation'] == 'log10'
 
+    def test_log10_observable_keeps_its_linear_base_distribution(self, tmp_path):
+        # petab1to2 folds the v1 transformation into noiseDistribution: petab < 0.9.0 left it
+        # blank (a missing return), petab >= 0.9.0 substitutes the natural-log family
+        # (log10-normal -> log-normal, with a warning). Either way the converter must state
+        # the scale once -- in the re-injected column -- over the v1 base family, or the
+        # importer refuses log10 stacked on log-normal as a contradiction (#679).
+        yaml = _write_v1_problem(tmp_path / 'v1', 'log10')
+        v2_yaml = petab1to2_preserve_scale(str(yaml), str(tmp_path / 'v2'))
+        obs = pd.read_csv(v2_yaml.parent / 'observables.tsv', sep='\t')
+        row = obs.set_index('observableId').loc['obs_V']
+        assert row['noiseDistribution'] == 'normal'
+
     def test_linear_observable_gets_no_transformation(self, tmp_path):
         # A lin observable needs no re-injection (lin is the v2 default) -> no column added,
         # so the converted problem stays byte-identical to plain petab1to2 on the observables.
@@ -230,6 +242,33 @@ def test_float64_nan_column_is_coerced_not_raised(self):
         inject_observable_transformations(df, {'obs_V': 'log10'})  # must not raise
         assert df.set_index('observableId').loc['obs_V', 'observableTransformation'] == 'log10'
 
+    def test_folded_log_normal_is_reset_to_the_v1_base(self):
+        # The petab >= 0.9.0 shape: the transformation already folded into noiseDistribution
+        # as the natural-log family. With the v1 base supplied, the log observable's row is
+        # reset to it and the linear observable's row is left exactly as petab1to2 wrote it.
+        df = self._petab1to2_obs_shape()
+        df['noiseDistribution'] = ['log-normal', 'normal']
+        inject_observable_transformations(df, {'obs_V': 'log10'}, {'obs_V': 'normal'})
+        by_id = df.set_index('observableId')
+        assert by_id.loc['obs_V', 'noiseDistribution'] == 'normal'
+        assert by_id.loc['obs_V', 'observableTransformation'] == 'log10'
+        assert by_id.loc['obs_lin', 'noiseDistribution'] == 'normal'
+
+    def test_folded_log_laplace_is_reset_to_laplace(self):
+        # The base family is the v1 author's, not always normal: log10 + laplace comes back
+        # from petab1to2 as log-laplace and must return to laplace under the log10 column.
+        df = self._petab1to2_obs_shape()
+        df['noiseDistribution'] = ['log-laplace', '']
+        inject_observable_transformations(df, {'obs_V': 'log10'}, {'obs_V': 'laplace'})
+        assert df.set_index('observableId').loc['obs_V', 'noiseDistribution'] == 'laplace'
+
+    def test_without_distributions_the_column_is_untouched(self):
+        # The pre-#679 contract: callers that pass no base map get only the transformation.
+        df = self._petab1to2_obs_shape()
+        df['noiseDistribution'] = ['log-normal', '']
+        inject_observable_transformations(df, {'obs_V': 'log10'})
+        assert df.set_index('observableId').loc['obs_V', 'noiseDistribution'] == 'log-normal'
+
 
 class TestHelpers: