-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add ZDC EtSum
s to Stage-2 L1T emulation
#45712
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45712/41350 |
A new Pull Request was created by @missirol for master. It involves the following packages:
@aloeliger, @antoniovilela, @cmsbuild, @davidlange6, @epalencia, @fabiocos, @mandrenguyen, @rappoccio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Ideally, this fix would enter |
+l1 |
The tests are stuck, but this appears to be just the comparisons, which I don't think are likely to be useful (L1 signed already anyway). If so I think we can go ahead and merge this before building pre7. |
Yes. I don't know enough to comment further, but it looks like what is in the configuration is intentional, and this PR is not modifying the value which was there before, so nothing else to do in this PR. |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. |
merge |
Hi,
The file is generated by the fillDescriptions code,
https://cmssdt.cern.ch/lxr/source/L1Trigger/L1TZDC/plugins/L1TZDCProducer.cc#0164
and appears under ../cfipython .
…On Fri, 30 Aug 2024, Andrew Loeliger wrote:
@aloeliger commented on this pull request.
> @@ -0,0 +1,7 @@
+import FWCore.ParameterSet.Config as cms
+
+from L1Trigger.L1TZDC.l1tZDCProducer_cfi import l1tZDCProducer as _l1tZDCProducer
@missirol I am working through a separate packer problem right now, but running into some ZDC things I don't understand and I'm trying to chase it down to the base of the ZDC emulation, which you must recently touched in this PR. I'm confused by this import (and how I'm not getting python problems) because `L1Trigger.L1TZDC.l1tZDCProducer_cfi` doesn't seem to exist. A quick search on github doesn't seem to reveal any mention of this file other than right here, and I can't find it under L1Trigger/L1TZDC.
This PR removed what I remembered as the previous L1Trigger ZDC emulation config `etSumZdcProducer_cfi`, was this perhaps supposed to be a rename of that configuration that didn't end up in the PR? Am I missing some other thing that was done to the ZDC emulation to make this import and producer work?
--
Reply to this email directly or view it on GitHub:
#45712 (review)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
Right, thanks. Sorry for the extra noise. |
PR description:
This PR adds the computation of ZDC
EtSum
s to the sequence used for the emulation of the Stage-2 L1T results.This addresses one of the issues discussed in #43214 (specifically, the L1T-ZDC not firing when using
L1REPACK:Full
).See the validation section below for some additional details.
PR validation:
The test in [1] outputs the L1T results (for a few events, using both real data and MC) in three cases: (a) before any re-emulation, (b) re-emulating only the L1-uGT, and (c) re-emulating L1T with
L1REPACK:Full
(data) orL1REPACK:FullMC
(MC).L1REPACK:Full
). In this case, the results of (c) are similar to (a/b), but not exactly the same. Solving this discrepancy is beyond the scope of this PR (it is left to L1T and ZDC experts).[1]
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:
N/A