Geant4 sensitive detectors revision 5

[civanch]

This PR is made instead of #21964.
Geant4 calorimeter sensitive detectors are updated:

1. In the base class CaloSD Geant4 interface ProcessHits(..) is implemented. In the 
    derived classes two main methods for sub-detectors : getFromLibrary(..) and 
    getEnergyDeposit(.. ) allow implementation of shower libraries/parametrisation 
    or detailed simulation 
   

2. Removed duplicated codes from sub-detector classes and use base class CaloSD methods 
   

3. use const G4Step* pointer in protected and private interfaces, use const pointers and 
    references for all other objects, where possible
   

4. use std::unique_ptr, auto, and other C++11 features where possible
   

5. reduce number of protected class members and reduced number of type conversions
   

6. optimized ECalSD run time methods - perform only one search over map of logical 
    volumes instead of doing this in each method
   

7. remove usage of G4ParticleTable at initialisation use instead static utility
   

8. optimised LogInfo and LogDebug - verbose output is substantially smaller but keeping
    similar information
   

9. removed old commented lines
   

10. applied corrections suggested by @kpedro88 and @VinInn to previous PRs

Correctness of this PR means identical results for all WFs.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23226/4695

[cmsbuild]

A new Pull Request was created by @civanch (Vladimir Ivantchenko) for master.

It involves the following packages:

SimG4CMS/Calo
SimG4CMS/CherenkovAnalysis
SimG4CMS/EcalTestBeam
SimG4CMS/Forward
SimG4CMS/HGCalTestBeam
SimG4CMS/HcalTestBeam
SimG4CMS/ShowerLibraryProducer

@cmsbuild, @civanch, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks.
@makortel, @argiro this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[civanch]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28007/console Started: 2018/05/17 12:51

[cmsbuild]

-1

Tested at: 62841b9

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23226/28007/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

• RelVals:

When I ran the RelVals I found an error in the following worklfows:
20034.0 step2

runTheMatrix-results/20034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17/step2_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17.log

20434.0 step2

runTheMatrix-results/20434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D19_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D19+RecoFullGlobal_2023D19+HARVESTFullGlobal_2023D19/step2_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21.log

21234.0 step2

runTheMatrix-results/21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21/step2_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21.log

[cmsbuild]

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

[kpedro88]

Somehow the changes here cause a crash in CaloTruthAccumulator::fillSimHits()

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28201/console Started: 2018/05/25 19:26

[cmsbuild]

Pull request #23226 was updated. @cmsbuild, @civanch, @kpedro88, @mdhildreth can you please check and sign again.

[cmsbuild]

+1
Tested at: 8355039
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23226/28201/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 31
• DQMHistoTests: Total histograms compared: 2901706
• DQMHistoTests: Total failures: 2
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2901514
• DQMHistoTests: Total skipped: 190
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
• Checked 128 log files, 14 edm output root files, 31 DQM output files

[kpedro88]

+1

[civanch]

+1

[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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

[fabiocos]

@civanch it is an impressive piece of work. Did you check the simulation performances after this cleaning?

One minor point that I observe, we have many #define/ifdef DebugLog instructions in the touched classes, could we take the opportunity of this big migration to modernize this to #define/ifdef EDM_ML_DEBUG , as I agreed with @kpedro88 to do for the MTD part?

[civanch]

@fabiocos , I would guess, more cleanup of LogDebug/LogInfo/LogVerbatim will be useful. #define/ifdef is the Sunanda style to keep track on the code, for me LogDebug would be enough but I would do this after the discussion. After cleanup of remaining Forward/muon SD classes it would be possible to drop more protected class members in CaloSD and in that iteration improve debug printouts.

The performance should be checked after PR is merged, because CPU checks are specific - the condition should be stable. I would expect ~1% improvement for a standard WF, more useful may be for HGCAL, which is critical from CPU point of view.

[fabiocos]

@civanch I do not think it is terribly urgent, if you have a general review of the verbosity in mind I am fine in keeping it for a separate PR, but I think it would be good to do it. Using Sunanda's flags or the native LogDebug is basically similar from the point of view of the needed modifications and operations, the former may perhaps allow a more selective choice of the printouts, especially for such a large module as g4SimHits

[fabiocos]

+1

[fabiocos]

@civanch well, it depends on what is recompiled to enable LogDebug...