-
Notifications
You must be signed in to change notification settings - Fork 1
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 central phi functions instead LST ones #146
base: master
Are you sure you want to change the base?
Use central phi functions instead LST ones #146
Conversation
/run all |
There was a problem while building and running in standalone mode. The logs can be found here. |
There was a problem while building and running with CMSSW. The logs can be found here. |
@ariostas Sorry, I forgot, did we move to 15_0? Could you tell me the exact version, so that I can write it on our repo, and then update this PR appropriately? |
@VourMa I set it up so that now it always uses the latest release, so it's using 15_0_0_pre2. For some reason |
Oops, I didn't think of that. Feel free to just run the tests when you sort it out. Thank you for taking care of it! |
It should work now. /run all |
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots. The full set of validation and comparison plots can be found here. Here is a timing comparison:
|
The PR was built and ran successfully with CMSSW. Here are some plots. OOTB All TracksThe full set of validation and comparison plots can be found here. |
cmssw/RecoTracker/LSTCore/src/alpaka/NeuralNetwork.h Lines 41 to 48 in eec20cb
Can you remove this one in the inference code I added as well? Should probably check that it doesn't affect the performance plots. I tried using another implementation of the delta phi function and it gave some weird results, not sure if it was just a bug in my old code. |
eec20cb
to
42dd567
Compare
Sure, I replaced it in the new version I pushed. Let's see if it works out. |
/run all |
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots. The full set of validation and comparison plots can be found here. Here is a timing comparison:
|
The PR was built and ran successfully with CMSSW. Here are some plots. OOTB All TracksThe full set of validation and comparison plots can be found here. |
Looks good, thanks. |
This is the follow up for #142. I have created it as a draft PR, as, to be submitted to cms-sw, we would need to first merge #145 (for proper naming of the
reducePhiRange
functions) and #141 (so that we can have meaningful tests). Having said that, I think we can start discussing and testing this internally.