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

Warnings for scipy and laplace prior. #1228

Merged
merged 5 commits into from
Dec 1, 2023

Conversation

PaulJonasJost
Copy link
Collaborator

@PaulJonasJost PaulJonasJost commented Nov 30, 2023

Closes #1189.

Error should only appear for priors? Otherwise could also throw a general error, but not sure how helpful that one would be.

pypesto/optimize/optimizer.py Outdated Show resolved Hide resolved
PaulJonasJost and others added 2 commits November 30, 2023 16:20
Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2023

Codecov Report

Attention: 1768 lines in your changes are missing coverage. Please review.

Comparison is base (160c2a8) 88.16% compared to head (223ed7f) 71.92%.
Report is 454 commits behind head on develop.

Files Patch % Lines
pypesto/hierarchical/optimal_scaling/solver.py 9.95% 362 Missing ⚠️
...ypesto/hierarchical/spline_approximation/solver.py 12.15% 253 Missing ⚠️
pypesto/hierarchical/optimal_scaling/problem.py 16.74% 179 Missing ⚠️
pypesto/hierarchical/petab.py 0.00% 149 Missing ⚠️
pypesto/hierarchical/inner_calculator_collector.py 17.12% 121 Missing ⚠️
...pesto/hierarchical/spline_approximation/problem.py 20.94% 117 Missing ⚠️
pypesto/hierarchical/problem.py 23.07% 100 Missing ⚠️
pypesto/ensemble/ensemble.py 69.36% 68 Missing ⚠️
pypesto/hierarchical/solver.py 21.68% 65 Missing ⚠️
pypesto/hierarchical/util.py 17.10% 63 Missing ⚠️
... and 19 more

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

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #1228       +/-   ##
============================================
- Coverage    88.16%   71.92%   -16.24%     
============================================
  Files           79      148       +69     
  Lines         5257    11863     +6606     
============================================
+ Hits          4635     8533     +3898     
- Misses         622     3330     +2708     

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

@PaulJonasJost PaulJonasJost merged commit fd64c8f into develop Dec 1, 2023
18 checks passed
@dweindl dweindl linked an issue Dec 1, 2023 that may be closed by this pull request
@dilpath
Copy link
Member

dilpath commented Dec 1, 2023

How does this affect optimization? i.e. does the gradient then become nan when the parameter is at the mean of the Laplace, even if there are finite contributions from the likelihood? If so, then it might be better to define the gradient contribution from the Laplace at the mean as 0, so that the optimizer can still use the gradient information from the likelihood.

@dweindl dweindl deleted the add_warning_scipy_priors branch December 1, 2023 18:57
@dweindl
Copy link
Member

dweindl commented Dec 1, 2023

i.e. does the gradient then become nan when the parameter is at the mean of the Laplace, even if there are finite contributions from the likelihood?

Yes. I am not sure whether this is really a problem in practice. I'd guess it's rather unlikely that the optimizer steps exactly on the mean.

If so, then it might be better to define the gradient contribution from the Laplace at the mean as 0, so that the optimizer can still use the gradient information from the likelihood.

Also fine with me.

This was referenced Dec 5, 2023
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.

test/base/test_prior.py ValueError: array must not contain infs or NaNs
4 participants