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

Should we use 64 bit float to represent event weights, instead of 32 bit? #349

Open
SimonHeybrock opened this issue Jul 7, 2022 · 3 comments

Comments

@SimonHeybrock
Copy link
Member

scn.load and scn.load_nexus currently use float32 to repsent the event weights. These all default to 1.0 so this is fine. However, when summing events later the user may encounter a surprising precision loss.

We should carefully consider the tradeoff between risk for bugs and memory use. Reduction operations use double precision for intermediate values so many problems are avoided. Nevertheless final results can be affected significantly. For example:

import numpy as np
np.float32(75893996)

gives 75894000.

@nvaytet
Copy link
Member

nvaytet commented Jul 18, 2022

Reduction operations use double precision for intermediate values

Does this mean that if we sum all the events in a bin, then the sum correctly produces 75893996, but then it converts it back to float32 just before returning to match the dtype of the initial Variable, and this is where it goes wrong?

We could consider always make reduction operations return float64, but that behaviour may be surprising?

@SimonHeybrock
Copy link
Member Author

Yes, that is where it goes wrong.

We could consider always make reduction operations return float64, but that behaviour may be surprising?

I think we had this discussion and decided against at that point, but we may reconsider.

@nvaytet
Copy link
Member

nvaytet commented Jul 19, 2022

I guess this is something surprising to developers, but maybe the vast majority of the users will not care about (or even notice) the difference between 32 and 64 bit?
Or at least they will care less about it than getting the wrong result?

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

No branches or pull requests

2 participants