-
Notifications
You must be signed in to change notification settings - Fork 20
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 narrowing conversion in QIEDecoder #1426
Conversation
8085850
to
d380e93
Compare
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.
Overall this looks good to me, but since there is no CI test for testbeam data stuff, have you tested this manually or just ticked the box?
I compiled and run the configs in the validation. What should I have run to test it on testbeam data? |
Is this it? https://github.com/LDMX-Software/ldmx-sw/blob/trunk/TrigScint/exampleConfigs/runQIEDecode.py |
Well I ran that file, ran into an issue #1427 and fixed it #1429 Now when I run I get
which makes me think that it's technically working. If you have a better input file I can run on that too. Maybe I should tag @hherde too. |
OK so running
works, which makes me think that this is enough to merge this. |
I can't tell from the output if what you ran actually did anything. But I'm rather confident your change is fine. Making an issue for setting up tests for our data workflow is a good outcome of this PR. |
I am updating ldmx-sw, here are the details.
What are the issues that this addresses?
Resolves the last part of #1199
by statis casting.
Check List
Resolves #1199