-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Addressing ECAL local reconstruction on GPU issues #33116
Addressing ECAL local reconstruction on GPU issues #33116
Conversation
…ndFormats/EcalObjects.
…ormats/EcalObjects.
…ndFormats/EcalObjects.
…Formats/EcalObjects.
…dFormats/EcalObjects.
…ormats/EcalObjects.
…ormats/EcalObjects.
… CondFormats/EcalObjects.
…ndFormats/EcalObjects.
…dFormats/EcalObjects.
…ndFormats/EcalObjects.
…dFormats/EcalObjects.
…ique and cms::cuda::copyAsync.
…ique and cms::cuda::copyAsync. Part 2.
…ique and cms::cuda::copyAsync. Part 3.
…lDigi/interface/EcalMGPASample.h instead.
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e8dd18/13573/summary.html Comparison SummarySummary:
|
Thx for the updates. As far as I see, this is the pending question before the reco sig. I can also run a comparison myself, I was just wondering if it was already done. Thanks!
|
The additional GPU tests seem to consist of WFs 10824.502 and 10824.512 and they passed. However, it looks like the two WFs were not included in the comparisons (https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_3_X_2021-03-16-2300+e8dd18/41754/). Is that the normal behaviour? |
There is no automatic comparison of GPU workflows at the moment, it needs to be implemented. I can run a check wrt. the baseline (or if you already did, let me know). |
I am running a comparison with 10824.512 atm. |
Comparing WF 10824.512 ECAL sorted RecHits from CMSSW_11_3_0_pre3 with the ones from CMSSW_11_3_0_pre3 + this PR on 100 events, I get the same number of RecHits with the same mean and std dev. values for all variables. For EB and EE. So this looks like there are no changes in the output, as expected. |
+reconstruction
|
@cms-sw/db-l2 , @cms-sw/alca-l2 do you have questions or comments? |
@cms-sw/db-l2 @cms-sw/alca-l2 |
+1 |
kind reminder @cms-sw/alca-l2 |
+1
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
This PR addresses some issues that have been left open during the initial PR #31719 . They are tracked in issue #32480 .
The fixed issues:
PR validation:
Passes matrix WFs 10824.512 and 11634.512.
No changes in the RecHits output.