search

PEtab-dev / libpetab-python

Python package for working with PEtab files
https://libpetab-python.readthedocs.io
MIT License
14 stars 6 forks source link

Treat observableParameter overrides as placeholders in noise formulae #231

Closed dilpath closed 9 months ago

dilpath commented 9 months ago

Currently, linting will fail if an observableParameter (e.g. a scaling or offset) appears in the noise formulae. Having the full observable formulae in the noise formulae is currently the only way to get observable-dependent noise -- this means noise formulae should support observableParameter placeholders.

This fixes the issue for this case: https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/pull/197

If I compare the AMICI model from these cases:

codecov-commenter commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (c0158ba) 76.28% compared to head (f907b2e) 76.28%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #231 +/- ## ======================================== Coverage 76.28% 76.28% ======================================== Files 34 34 Lines 3188 3188 Branches 774 774 ======================================== Hits 2432 2432 + Misses 555 554 -1 - Partials 201 202 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dilpath commented 9 months ago

Thanks. Makes sense. Could you please add/extend a test?

Done! I did not see a pre-existing test, so I created a new one in a test script that already provided a PEtab problem to work with. I changed this PEtab problem, because observable_1 is a parameter in the test problem model, and obs1 is the observable ID used in the measurements table.

We should probably also clarify that in https://github.com/PEtab-dev/PEtab/blob/main/doc/documentation_data_format.rst

Sounds good, done here: https://github.com/PEtab-dev/PEtab/pull/574