Skip to content

Fix invalid and lossy output of petab_math_str - #545

Merged
dweindl merged 4 commits into
PEtab-dev:mainfrom
dweindl:fix_math_printer
Oct 15, 2026
Merged

dweindl merged 4 commits into
PEtab-dev:mainfrom
dweindl:fix_math_printer

Conversation

@dweindl

@dweindl dweindl commented Oct 14, 2026

Copy link
Copy Markdown
Member

Make petab_math_str produce valid PEtab math expressions that round-trip, and use it when writing v2 condition and measurement tables.

PetabStrPrinter now:

  • prints &&, ||, !, ==, != instead of sympy's &, |, ~, Eq(...), Ne(...)
  • parenthesizes negative and non-integer rational bases of powers ((-2) ^ x was printed as -2.0 ^ x)
  • prints n-ary Min/Max as nested binary min/max
  • prints pi as its double value and E as exp(1)
  • prints double-precision floats with the shortest exact representation (0.6666666666666666 instead of 0.666666666666667), and integer-valued floats as integers (100*x instead of 100.0*x)
  • raises for Piecewise without a default condition instead of dropping the last condition

ConditionTable.to_df and MeasurementTable.to_df now use petab_math_str instead of str, so expressions are written in PEtab syntax and float values are not truncated.

Fixes #544.

🤖 Generated with Claude Code

Make `PetabStrPrinter` produce valid PEtab math that round-trips:

* print `&&`, `||`, `!`, `==`, `!=` instead of sympy's `&`, `|`, `~`,
  `Eq(...)`, `Ne(...)`
* parenthesize negative and non-integer rational bases of powers
* print n-ary `Min`/`Max` as nested binary `min`/`max`
* print `pi` as its double value and `E` as `exp(1)`
* print double-precision floats with the shortest exact representation,
  and integer-valued floats as integers
* raise for `Piecewise` without a default condition

Use `petab_math_str` instead of `str` in `ConditionTable.to_df` and
`MeasurementTable.to_df`.

Fixes PEtab-dev#544.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dweindl dweindl self-assigned this Oct 14, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.74%. Comparing base (539333b) to head (358a4f1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #545      +/-   ##
==========================================
+ Coverage   77.58%   77.74%   +0.16%     
==========================================
  Files          67       67              
  Lines        7673     7702      +29     
  Branches     1374     1378       +4     
==========================================
+ Hits         5953     5988      +35     
+ Misses       1231     1226       -5     
+ Partials      489      488       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dilpath dilpath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing!

Comment thread petab/v1/math/printer.py

def _print_Pi(self, expr):
"""PEtab has no pi constant; print its double-precision value"""
return repr(float(expr))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard code? No advantage to computing from expr I guess

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not introduce our own literal here. I find it easier to verify this way.

Comment thread tests/v2/test_core.py Outdated


def test_to_df_math_expressions():
"""Expressions are written in PEtab syntax and without precision loss."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if precision loss is tested here. e.g. there are no long numbers that could be rounded.

@dweindl dweindl Oct 15, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tested via 2 / 3, repr(2 / 3), but I'll make that more obvious.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I did not realize it would change a fraction to a decimal. Thanks for opening #548

dweindl and others added 2 commits October 15, 2026 10:32
@dweindl
dweindl merged commit 663b383 into PEtab-dev:main Oct 15, 2026
7 checks passed
@dweindl
dweindl deleted the fix_math_printer branch October 15, 2026 08:52
dweindl added a commit to AMICI-dev/AMICI that referenced this pull request Oct 15, 2026
Fix `test_flatten_timepoint_specific_output_overrides` for the current
libpetab-python `main`.

Since PEtab-dev/libpetab-python#545, `petab_math_str` prints
double-precision floats such that they round-trip, e.g., `1` instead of
`1.00000000000000`. The flattened observable IDs are derived from the
override values, so they are now
`obs1__obsParOverride1_1__noiseParOverride1` instead of
`obs1__obsParOverride1_1_00000000000000__noiseParOverride1`. This test
fails on `main` since then.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
dweindl added a commit to PEtab-dev/petab_test_suite that referenced this pull request Oct 19, 2026
libpetab now writes all standard columns of v2 tables by default
(PEtab-dev/libpetab-python#543), which added an empty `parameterName`
column to all parameter tables. Load v2 case modules under
`petab.v2.option_context(table_columns="non_empty")` to omit columns
that are empty in all rows.

Regenerate the v2 cases. Apart from dropped empty columns, the only
changes are due to the updated number formatting in math expressions
(PEtab-dev/libpetab-python#545), e.g. `0.500000000000000` -> `0.5`.
Solution files are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dweindl added a commit to PEtab-dev/petab_test_suite that referenced this pull request Oct 19, 2026
libpetab now writes all standard columns of v2 tables by default
(PEtab-dev/libpetab-python#543), which added an empty `parameterName`
column to all parameter tables. Load v2 case modules under
`petab.v2.option_context(table_columns="non_empty")` to omit columns
that are empty in all rows.

Regenerate the v2 cases. Apart from dropped empty columns, the only
changes are due to the updated number formatting in math expressions
(PEtab-dev/libpetab-python#545), e.g. `0.500000000000000` -> `0.5`.
Solution files are unchanged.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

petab_math_str and v2 to_df produce invalid or lossy PEtab math expressions

3 participants