Memory efficient implementation of timing information for HGCDigitizer module for Premix Workflow.

[adas1994]

PR description:

<The HGCDigitizer module, in it's current form, store timing information of all the incoming hits from the minbias events in PreMixStage1 of the Premix Workflow. This PR aims to drop the timing information from redundant hits from the minbias events, once a HGCal cell reaches it's fire-off threshold of accumulated charge.

I found the PR to improve the performance of the code in terms of CPU performance, reduced memory and disk usage. The quantitative figures are mentioned below -
CPU Performance -
For BaseLine Igprof summary, I got the following

   88.1  ...............  211'531'179'858 / 211'531'179'858    1'008'942'599 / 1'008'942'599      edm::BMixingModule::produce(edm::Event&, edm::EventSetup const&) [16]
    2.3  ...............    5'484'689'754 / 5'484'689'754             13'591 / 13'591             edm::PoolOutputModule::write(edm::EventForOutput const&) [201]

###############################################################################
And for the new Premix method, I got

   80.8  ...............  109'380'472'943 / 109'380'472'943      957'052'716 / 957'052'716        edm::BMixingModule::produce(edm::Event&, edm::EventSetup const&) [16]
    2.2  ...............    2'970'142'735 / 2'970'142'735             11'298 / 11'298             edm::PoolOutputModule::write(edm::EventForOutput const&) [280]

Disk Usage -

PHGCSimAccumulator_simHGCalUnsuppressedDigis_EE_DIGI. 2.57756e+08 4.29255e+06
PHGCSimAccumulator_simHGCalUnsuppressedDigis_HEfront_DIGI. 3.5771e+07 621998
PHGCSimAccumulator_simHGCalUnsuppressedDigis_HEback_DIGI. 7.30215e+06 119472

For my new method, disk usage

PHGCSimAccumulator_simHGCalUnsuppressedDigis_EE_DIGI. 6.85587e+07 4.08996e+06
PHGCSimAccumulator_simHGCalUnsuppressedDigis_HEfront_DIGI. 6.34798e+06 333652
PHGCSimAccumulator_simHGCalUnsuppressedDigis_HEback_DIGI. 760055 51035.3

MemoryInfo-

FOr the addPileup method, the new proposed procedure uses almost an order of magnitude less memory(as shown in the attached screenshot; Black background screenshot- Baseline method, White background screenshot- My new method

)

PR validation:

<I ran a validation test by comparing results from WorkFlow # 20634.0(standard mix) and # 20634.99(premix) using custom validation package here - https://github.com/amartelli/HGCTimingAnalysis/tree/TimingAnalysis_11_1_X >

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30867/17227

• This PR adds an extra 24KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File SimCalorimetry/HGCalSimProducers/src/HGCDigitizer.cc modified in PR(s): Phase2-hgx256 Try to provide V14 geometry for HGCal #30862

[cmsbuild]

A new Pull Request was created by @adas1994 for master.

It involves the following packages:

SimCalorimetry/HGCalSimProducers

@cmsbuild, @civanch, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks.
@vandreev11, @sethzenz, @makortel, @rovere, @lgray, @cseez, @apsallid, @pfs, @deguio, @hatakeyamak this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

[kpedro88]

please test

[cmsbuild]

The tests are being triggered in jenkins.

• CMSSW_11_2_X_2020-07-21-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/8206/console Started: 2020/07/21 23:38

[cmsbuild]

-1

Tested at: d71814e

CMSSW: CMSSW_11_2_X_2020-07-21-1100
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d454d5/8206/summary.html

I found follow errors while testing this PR

Failed tests: ClangBuild

• Clang:

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

[cmsbuild]

Comparison not run due to Build errors/Fireworks only changes/No short matrix requested (RelVals and Igprof tests were also skipped)

[kpedro88]

I ran the workflow to make sure it would run successfully, but without adding it to the default PR tests, the baseline plots aren't stored, so the comparison plots can't be generated.

[adas1994]

I ran the workflow to make sure it would run successfully, but without adding it to the default PR tests, the baseline plots aren't stored, so the comparison plots can't be generated.

Okay, so I am guessing that not much left here to do on my part in this PR. Btw, @kpedro88 I have a question not necessarily related to this PR. Is it mainstream that developers look for open issues in CMSSW github and pick up issues to work on by their own, or does one have to go through some-kind-of-approval-process to work on something? For example, if I want to work on Open Issue #30968 , do I need to talk to people or am I allowed to just get started ?

[kpedro88]

Anyone is welcome to work on any open issue*. To avoid duplicating effort, if you are starting to work on a certain issue, you may want to add a comment to that effect (so others are aware).

* of course, if the solution has some impact on physics performance, it may require validation and approval by the appropriate DPG or POG, as usual.

[adas1994]

@adas1994 I'm going to add a premixing workflow to the regular PR tests. This will produce the comparison plots automatically. Hopefully that can all converge in the next ~day, and then I will be able to approve this PR.

Any update on this, @kpedro88 ?

[kpedro88]

The PR for that is #30988. It became a bit more involved than initially expected, but hopefully it can be merged soon.

[kpedro88]

please test

[cmsbuild]

The tests are being triggered in jenkins.

• CMSSW_11_2_X_2020-08-04-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/8582/console Started: 2020/08/05 16:02

[cmsbuild]

+1
Tested at: 66cc323
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d454d5/8582/summary.html
CMSSW: CMSSW_11_2_X_2020-08-04-2300
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d454d5/8582/summary.html

Comparison Summary:

• No significant changes to the logs found
• ROOTFileChecks: Some differences in event products or their sizes found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 2612401
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2612352
• DQMHistoTests: Total skipped: 48
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 149 log files, 22 edm output root files, 35 DQM output files

[kpedro88]

+upgrade
changes in premix workflow output not detectable at the 10 event level

[cmsbuild]

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[silviodonato]

+1