-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Enabling SiPixel digi morphing from 2025 onward #49065
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
Enabling SiPixel digi morphing from 2025 onward #49065
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49065/46295
|
|
A new Pull Request was created by @ferencek for master. It involves the following packages:
@cmsbuild, @davidlange6, @fabiocos, @ftenchini, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild, please test |
|
looking thru the log it seems these unit tests are failing: |
|
indeed (see log): |
|
so the client
takes it from here:
this comes from the auto generated file -- without getting changed from the process modifier. Whereas:
gets it from here: cmssw/RecoLocalTracker/SiPixelClusterizer/python/SiPixelClusterizerPreSplitting_cfi.py Lines 10 to 13 in dd7156a
So there is a mismatch, which explains why the relvals work fine, where the clients do not. on second thought, this perhaps would break some of the patatrack workflows, to be tested. |
|
-1 Failed Tests: UnitTests Unit TestsI found 2 errors in the following unit tests: ---> test TestDQMOnlineClient-beam_dqm_sourceclient had ERRORS ---> test TestDQMOnlineClient-beampixel_dqm_sourceclient had ERRORS Comparison SummarySummary:
|
|
+1 Size: This PR adds an extra 28KB to repository Comparison SummarySummary:
AMD_MI300X Comparison SummarySummary:
AMD_W7900 Comparison SummarySummary:
NVIDIA_H100 Comparison SummarySummary:
NVIDIA_L40S Comparison SummarySummary:
NVIDIA_T4 Comparison SummarySummary:
|
|
@cms-sw/dqm-l2 your sig was added after fixing an issue with the unit tests. |
|
+1 |
|
Taking a look now |
|
From the comparison failures, I see several histograms not being filled, e.g., these here, or here. Is this expected? Edit: P5 tests are OK, if the differences are expected (I cannot really judge with my limited knowledge), we can sign it. |
if duplicates vanish it's a good thing (in principle). |
|
+dqm
|
|
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 be automatically merged. |
just adding for completeness, there was an entire release validation check about this link to ValDB (including re-reco of an entire data-taking era link). I would not call this "rushed check of histos".... |
I was referring to the check that I just did hastily, I had no knowledge of the work that you mentioned. |
Sure, I commented for the record, to dispel the impression this got merged in a rush without detailed checks on the physics outcome. |
PR description:
This PR enables the SiPixel digi morphing in the offline reconstruction from 2025 onward. The HLT version of digi morphing running on GPUs has been introduced in #48734.
PR validation:
None
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
To be backported to CMSSW_15_1_X and CMSSW_15_0_X.