-
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
(Un)packers for hadronic showers at the uGMT input and output #35627
(Un)packers for hadronic showers at the uGMT input and output #35627
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35627/25899
|
A new Pull Request was created by @dinyar (Dinyar Rabady) for master. It involves the following packages:
@cmsbuild, @rekovic, @cecilecaillol can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
-1 Failed Tests: Build ClangBuild BuildI found compilation warning when building: See details on the summary page. Clang BuildI found compilation warning while trying to compile with clang. Command used:
See details on the summary page. |
59fa1a6
to
3a8d88c
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35627/25923
|
Pull request #35627 was updated. @cmsbuild, @rekovic, @cecilecaillol can you please check and sign again. |
please test |
-1 Failed Tests: AddOn AddOn Tests
Expand to see more addon errors ...Comparison SummarySummary:
|
+1 |
+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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
this breaks #36396 |
I suspect 9960aaa caught the other PR out. If that's the issue I can commit a fix for it in the next hour or two I'd say. |
Also https://github.com/cms-sw/cmssw/pull/36018/files#diff-a2194e73035b8a64c72a7363af81ce438a545aca33b67ce5f616ff6ed98b4d0cR15 and https://github.com/cms-sw/cmssw/pull/36018/files#diff-a2194e73035b8a64c72a7363af81ce438a545aca33b67ce5f616ff6ed98b4d0cR16 , the issue being the removal here of the two methods
which should be restored. |
As @dinyar is already fixing the conflict by encapsulating |
This is what I suggested as the quickest and robust fix to be merged before next IB. |
Agree. Thanks! |
PR description:
This PR extends the
RegionalMuon
andMuon
unpackers to be able to also unpackRegionalMuonShower
andMuonShower
objects respectively. This functionality is gated by the expected uGMT firmware version read from the readout payload.I've also modified the
RegionalMuonShower
interface to more closely mirror that of theRegionalMuonCand
data format.PR validation:
Ran
scram build code-checks
,runTheMatrix.py -l limited -i all --ibeos
, andscram b runtests
locally.attn @rekovic, @kbunkow, @dildick