PetabSimulationResult for PetabSimulator.simulate

[dweindl]

Introduce dataclass PetabSimulationResult for PetabSimulator.simulate results. I think this will be more convenient: compatible with type annotations, static analysis, and auto-completion.

Closes #3094.

[codecov]

Codecov Report

❌ Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.60%. Comparing base (8603c1e) to head (316939f).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
python/sdist/amici/sim/sundials/petab/_v2.py	80.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3104      +/-   ##
==========================================
- Coverage   78.25%   77.60%   -0.66%     
==========================================
  Files         315      315              
  Lines       20464    20478      +14     
  Branches     1500     1500              
==========================================
- Hits        16014    15891     -123     
- Misses       4441     4578     +137     
  Partials        9        9              

Flag	Coverage Δ
cpp	71.74% <55.55%> (-0.01%)	⬇️
cpp_python	37.76% <55.55%> (+0.02%)	⬆️
petab	46.49% <70.00%> (+0.02%)	⬆️
petab_sciml	13.78% <0.00%> (-0.02%)	⬇️
python	70.37% <83.33%> (+<0.01%)	⬆️
sbmlsuite-jax	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
python/sdist/amici/adapters/fiddy.py	89.56% <100.00%> (ø)
python/sdist/amici/sim/sundials/petab/__init__.py	100.00% <ø> (ø)
python/tests/petab_/test_petab_v2.py	100.00% <100.00%> (ø)
python/sdist/amici/sim/sundials/petab/_v2.py	93.20% <80.00%> (-0.87%)	⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[FFroehlich]

Why would these be modified? I was wondering whether it would make sense to have dataclass(frozen=True)

[dweindl]

This depends on how the ExpDatas are handled. For performance/memory reasons, it would be preferable to create them once, and store them in the ExperimentManager, and then only update the parameters when necessary.
We can then either always copy them when creating the PetabSimulationResult (which means we could also just not re-use them in the first place), or we only store the reference here, meaning that after the next simulation, the parameters may have changed.
Ideally, we'd split up ExpData into a parameter-dependent and parameter-independent part to circumvent this problem. That would however be a bigger thing (#3106).

The frozen=True decision is independent and would have only little effect because all current members are mutable. No objections to applying that, though.

[FFroehlich]

This depends on how the ExpDatas are handled. For performance/memory reasons, it would be preferable to create them once, and store them in the ExperimentManager, and then only update the parameters when necessary. We can then either always copy them when creating the PetabSimulationResult (which means we could also just not re-use them in the first place), or we only store the reference here, meaning that after the next simulation, the parameters may have changed. Ideally, we'd split up ExpData into a parameter-dependent and parameter-independent part to circumvent this problem. That would however be a bigger thing.

The frozen=True decision is independent and would have only little effect because all current members are mutable. No objections to applying that, though.

Oh, I see. Not great, but makes sense.

[dweindl]

Agreed.