Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hierarchical: avoid recomputing inner parameters if simulation failed #1426

Merged
merged 4 commits into from
Jul 4, 2024

Conversation

dilpath
Copy link
Member

@dilpath dilpath commented Jul 3, 2024

result.x = None if simulation fails at the startpoint. result.x is used to compute hierarchical inner parameters after optimization. When result.x = None, an AMICI simulation to compute inner parameters is still run, unexpectedly... I didn't look deeper there, but this PR stops the computation of inner parameters if simulation fails at the startpoint (defined by result.x = None).

This PR means users do not see NaN in p AMICI simulation errors due to result.x = None. This can be confusing for users, who then try (and fail) to reproduce those errors with objective(result.x0).

@codecov-commenter
Copy link

codecov-commenter commented Jul 3, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.34%. Comparing base (10d6092) to head (ef1ac7a).

Files Patch % Lines
pypesto/objective/amici/amici.py 80.00% 1 Missing ⚠️

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

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1426   +/-   ##
========================================
  Coverage    83.33%   83.34%           
========================================
  Files          161      161           
  Lines        13235    13237    +2     
========================================
+ Hits         11030    11032    +2     
  Misses        2205     2205           

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

Copy link
Member

@dweindl dweindl left a comment

Choose a reason for hiding this comment

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

Makes sense.

pypesto/objective/amici/amici.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Doresic Doresic left a comment

Choose a reason for hiding this comment

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

Good change. Thanks!

@dilpath dilpath merged commit ef34eeb into develop Jul 4, 2024
18 checks passed
@dilpath dilpath deleted the avoid_failed_inner_calculation branch July 4, 2024 10:24
This was referenced Jul 25, 2024
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.

4 participants