ICB-DCM / pyPESTO

python Parameter EStimation TOolbox
https://pypesto.readthedocs.io
BSD 3-Clause "New" or "Revised" License
216 stars 47 forks source link

Hierarchical optimization: fix unnecessary simulation #1327

Closed dweindl closed 5 months ago

dweindl commented 5 months ago

Currently model simulation is run twice, even when no sensitivities are requested and the optimal inner parameters could be applied directly.

codecov-commenter commented 5 months ago

Codecov Report

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

Project coverage is 84.46%. Comparing base (2558763) to head (e2501a9).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #1327 +/- ## =========================================== + Coverage 84.42% 84.46% +0.03% =========================================== Files 153 153 Lines 12529 12529 =========================================== + Hits 10577 10582 +5 + Misses 1952 1947 -5 ```

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

dweindl commented 5 months ago

To confirm we're thinking the same: the only thing this changes is that if the sensitivity method is adjoint, then the sensi_orders = (0,) calls of the objective will be called through self.calculate_directly(...) instead of self.call_amici_twice(...), right?

Correct.

It was necessary because checking for sensitivity method alone is insufficient. sensitivity method and sensitivity order jointly decide whether sensitivities are to be computed.