-
Notifications
You must be signed in to change notification settings - Fork 91
Fix for #634 #638
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
Fix for #634 #638
Conversation
… with exposure adjustments to match paper.
…ated plot_ptf_resid in gallery.
…nores it as it did originall
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #638 +/- ##
=======================================
Coverage 84.44% 84.44%
=======================================
Files 84 84
Lines 4823 4823
Branches 610 610
=======================================
Hits 4073 4073
Misses 538 538
Partials 212 212
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@danielfong-act thanks for pulling this together. small nitpick, the previous figure numbering of 3.11 doesn't match the numbering of the current version of the paper. |
|
I swore I fixed that |
|
do we need to wait for a fix to #639 ? |
|
Nah it's fine. The missing standard residual(s) would be the last, rightmost point on the origin and development graphs. It's an intermittent bug, so I'll update #639 to reflect that. |
henrydingliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exposure adjustment appears correct.
* Fix for #634 (#638) squashed commits as some the earlier commit were later reversed * Added exposure adjustment to barnzehn.py. Updated ptf example gallary with exposure adjustments to match paper. * fixed bug when not passing sample_weight to BarnettZehnwirth.fit. updated plot_ptf_resid in gallery. * removed sample_weight from barnettzehnwirth.fit * unremoved sample_weight from barnettzehnwirth.fit -- the estimator ignores it as it did originall * fixed graph title --------- Co-authored-by: danielfong-act <danielfong247@gmial.com> * adding another test for bz --------- Co-authored-by: danielfong-act <danielfong247@gmail.com> Co-authored-by: danielfong-act <danielfong247@gmial.com>
After adjusting the incrementals by their exposure in plot_ptf_resid.py, the resultant graphs match those of Barnett and Zehnwirth's paper (p. 281 of this paper). Exposures are given on the previous page. Also, the origin graph needed its axis limits tweaked. For reasons unknown, the data was stretched to the edges of the plot.
Thanks to @henrydingliu for noting that you don't need to construct a triangle to do triangle arithmetic; a numpy array works fine.