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
Fix consumes for phase2 premixing and reduce code duplication in EcalDigiProducer #22581
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22581/3914 |
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: SimCalorimetry/EcalSimProducers @cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@makortel , if fix the Producer why not to use GetByToken ? |
@civanch I'd prefer to leave that to the code authors. |
@civanch I think there's some reason for this having to do with pileup mixing, but don't remember exactly. |
+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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
AFAIK |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
The
consumes
for EB/EE/ESPCaloHit
s is controlled withdoEB
/doEE
/doES
, but thegetByLabel()
for signal and pileup events were called for all EB/EE/ES regardless of thedoEX
flags. In phase2 premixing stage2 (inDataMixingModule
/PreMixingModule
) these would lead to "get products without calling consumes" exception (I don't know why the regular mixing works), so this PR callsgetByLabel()
only for the enabled subdetectors.In addition I use the occasion to reduce the duplicated code in the constructors by removing almost everything from one of them and calling the other one from it. This change also fixes a bug in APD simulation that was fixed in #17300 for classical mixing for premixing (I presume this effect won't be seen in PR tests quoting from #17300 "the code that's not used in the standard sequences" (needed only if one wants to simulate ECAL barrel "spike" noise)".
Tested in CMSSW_10_1_X_2018-03-05-2300, no changes expected.
@mdhildreth @kpedro88