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

udate min fval #593

Merged
merged 3 commits into from
Mar 17, 2021
Merged

udate min fval #593

merged 3 commits into from
Mar 17, 2021

Conversation

FFroehlich
Copy link
Contributor

@FFroehlich FFroehlich commented Mar 13, 2021

This corrects the computation of min_val in the waterfall plot. This is a breaking change, since now log-plots yield use the standard offset_y=1-minval by default instead of an offst_y=0 by default. However, I believe this makes sense, since the previous implementation made it impossible to use the offset_y=1-minval, since this would always erroneously trigger

if (scale_y == 'log10') and (min_val + offset_y <= 0.):
. This implementation corrects this and the previous can still be restored by specifying offset_y=0.

@codecov-io
Copy link

codecov-io commented Mar 14, 2021

Codecov Report

Merging #593 (4b04495) into develop (160c2a8) will decrease coverage by 53.30%.
The diff coverage is 27.95%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop     #593       +/-   ##
============================================
- Coverage    88.16%   34.85%   -53.31%     
============================================
  Files           79       91       +12     
  Lines         5257     5726      +469     
============================================
- Hits          4635     1996     -2639     
- Misses         622     3730     +3108     
Impacted Files Coverage Δ
pypesto/ensemble/covariance_analysis.py 18.36% <0.00%> (-0.39%) ⬇️
pypesto/objective/aggregated.py 25.49% <0.00%> (-74.51%) ⬇️
pypesto/petab/importer.py 0.00% <0.00%> (-85.90%) ⬇️
pypesto/visualize/waterfall.py 14.63% <0.00%> (-81.67%) ⬇️
pypesto/store/read_from_hdf5.py 51.32% <5.55%> (-6.57%) ⬇️
pypesto/store/save_to_hdf5.py 18.96% <7.69%> (-43.18%) ⬇️
pypesto/objective/amici.py 20.32% <10.00%> (-69.51%) ⬇️
pypesto/visualize/misc.py 13.18% <11.11%> (-71.75%) ⬇️
pypesto/visualize/sampling.py 10.89% <12.42%> (-67.74%) ⬇️
pypesto/ensemble/ensemble.py 12.90% <22.22%> (-67.87%) ⬇️
... and 92 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc73d34...4b04495. Read the comment docs.

Copy link
Member

@yannikschaelte yannikschaelte left a comment

Choose a reason for hiding this comment

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

sounds fine for me. @paulstapor @DantongWang any concerns?

@FFroehlich FFroehlich merged commit 768842c into develop Mar 17, 2021
@FFroehlich FFroehlich deleted the fix_waterfall branch March 17, 2021 14:18
@yannikschaelte yannikschaelte mentioned this pull request Mar 17, 2021
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.

3 participants