-
Notifications
You must be signed in to change notification settings - Fork 21
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
Persist the actual simulated position of the ECAL hit rather than the cell center #1485
Conversation
OK I think it's worth doing this. |
e8ee960
to
ea2202d
Compare
X/Y makes a lot of sense. Z is what I believe is what's in #1490 |
@tomeichlersmith what do you think about the Z? Should I have #1490 takled in this PR too? Or maybe just open another PR... but then I dont know what to do with the weights |
I think it's prob better to be in another PR provided that we dont have too much time in between the two PRs. Then I can easily compare these plots to show the changes |
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.
Looks good!
In the spirit that we are about to fix the issue for Z in the branch |
… cell center (#1485) * Persist the actual simulated position of the ECAL hit rather than the cell center * Add Ecal simhit rechit position checks in DQM * Zoom in for Z axis
I am updating ldmx-sw, here are the details.
What are the issues that this addresses?
Resolves #1484
Resolves #1482
But I'm only opening it as a draft first to see the effect using the CI tests
Check List