-
Notifications
You must be signed in to change notification settings - Fork 35
Add docstring to explain the rounding in gather #1687
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
Conversation
Co-authored-by: David W.H. Swenson <david.swenson@omsf.io>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1687 +/- ##
==========================================
- Coverage 95.41% 93.01% -2.41%
==========================================
Files 185 185
Lines 16045 16045
==========================================
- Hits 15310 14924 -386
- Misses 735 1121 +386
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:
|
Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
IAlibay
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.
Thanks for adding this @dwhswenson !
| It has to be the uncertainty that determines the precision of the estimate, because if you said you had 3 significant figures in the estimate and used that to set the precision of the uncertainty, you'd have 12.3 +/- 0.0 -- no error at all! | ||
| We implement this by thinking of the decimal representation as "columns" centered on the decimal point. | ||
| We get the column index of the first non-zero number in the decimal representation of the uncertainty, and use the fact that ``np.round`` rounds to the number of decimal places you give it to report the estimate. |
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.
I might be misunderstanding what is being said here, but we only rely on np.round for cases where we go above decimal for rounding right? I.e. my understanding is that with negative columns we rely on string formatting, which is built-in round behaviour.
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.
Correct, we use string formatting to add trailing zeroes to the degree of precision we're reporting, which you can't do with floats and rounding.
@dwhswenson - correct?
|
No API break detected ✅ |
Requested by @atravitz : more explanation of the need for the weird rounding we do in gather.
(Does this need a news entry? Can do if required.)
Checklist
newsentryDevelopers certificate of origin