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

Use double to store energy in hits #193

Merged
merged 5 commits into from
Dec 11, 2024
Merged

Use double to store energy in hits #193

merged 5 commits into from
Dec 11, 2024

Conversation

ManuelHu
Copy link
Collaborator

So we do not pretend that we have more precision in output than we actually have...

@ManuelHu ManuelHu merged commit 1a1d1c3 into main Dec 11, 2024
7 checks passed
@ManuelHu ManuelHu deleted the double-precision branch December 11, 2024 13:01
@gipert
Copy link
Member

gipert commented Dec 11, 2024

Uhm this needs some more discussion, we have been using floats in the past to reduce disk space usage and I'm not sure we need double precision...

@tdixon97
Copy link
Collaborator

I guess we need double for positions and times but not energies?

@gipert
Copy link
Member

gipert commented Dec 11, 2024

We need doubles only if we need to store very small or very big numbers precisely, so this is not the case for positions (and also energy...?)

@tdixon97
Copy link
Collaborator

Could matter for positions, eg. for computing O(um) dead layer with detectors far from the origin of the coordinate system?

@ManuelHu
Copy link
Collaborator Author

This PR was not about storage and had no changes for the output files. We already have macro options to either use double or floats in output files.

This was only about the in-memory part: It did not really make sense to have the option to write a double to disk, if we do not even have a double in the first place...

@gipert
Copy link
Member

gipert commented Dec 11, 2024

Sorry I misunderstood, then no problem!

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