-
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
Simplify digi types for HGCal, add dummy unpacker and aliases #23703
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23703/5357 |
please test |
The tests are being triggered in jenkins. |
A new Pull Request was created by @kpedro88 (Kevin Pedro) for master. It involves the following packages: DataFormats/HGCDigi @perrotta, @cmsbuild, @civanch, @vazzolini, @kmaeshima, @mdhildreth, @dmitrijus, @nsmith-, @rekovic, @jfernan2, @kpedro88, @thomreis, @slava77, @vanbesien can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Comparison job queued. |
@kpedro88 a crash fastsim but apparently not in HGCal code... |
I don't see how it could be caused by this PR... there's no interaction between HGCal and fastsim yet |
Comparison is ready Comparison Summary:
|
When I ran the same test locally, it did not crash. This makes me suspect memory corruption somewhere. I'm running valgrind on that workflow now. (But again, I don't think it's actually related to this PR.) |
@kpedro88 I did the same test adding this PR onto this morning IB and got no crash, it could be unrelated, I agree |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
valgrind didn't show anything pertinent (though there is a potential memory leak, I'll post an issue). |
+1 large development affecting HGCal digitization and L1, effects only on Phase2 workflows. Backward compatibility with old production formats provided |
Followup to #23690.
While we will keep the old digi collection types in order to be able to read previously produced files, from now on all HGCal digis will just store a generic DetId. This is far preferable to adding switches throughout the code to handle the DetId type changes in the new geometry.
attn:
@bsunanda - I propagated this change to the testbeam code, is it acceptable?
@jbsauvan - I propagated this change to the L1T code, let me know if it interferes with anything (hopefully not, just a straightforward changing of types).
Followup:
In order to facilitate reading the old digi types from existing samples to rerun reco/validation sequences, a "dummy" unpacker was added for HGCal, with output type
hgcalDigis
. The unpacker module can be swapped out with a converter that changes the digi types, using the process modifierconvertHGCalDigisReco
.Similarly, to rerun digi/L1 sequences, an
EDAlias
was added for the simulated digis,simHGCalUnsuppressedDigis
. ThisEDAlias
can be swapped out with the same converter module using the process modifierconvertHGCalDigisSim
.