Sensitive detectors update 7

[civanch]

This is the last PR for clean up of Geant4 sensitive detector classes. Only forward detectors are updated:

1. A new class TimingSD surves for common computations. Duplicated codes in several classes are removed, instead inheritance is used.
2. Comments of @VinInn are implemented in a good part
3. Improved debug printouts
4. removed unused headers, commented and empty lines

Run-1 and run-2 workflows are expected to be unchanged.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23625/5258

[cmsbuild]

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

It involves the following packages:

SimG4CMS/Forward

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@makortel 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/28767/console Started: 2018/06/19 19:56

[cmsbuild]

+1
Tested at: a66a5a1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23625/28767/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 16 differences found in the comparisons
• DQMHistoTests: Total files compared: 31
• DQMHistoTests: Total histograms compared: 2902013
• DQMHistoTests: Total failures: 2
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2901821
• 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

[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 I understand that this is the latest code migration you are planning for 10_2_X. Concerning the timing detectors, MtdSD was almost a clone of FastTimerSD, that should be dropped as soon as we get rid of the old scenario. Anyway I fear that many, if not all, of these SD are not really tested in the short matrix (for instance not MTD). Did you check offline the identity of the output? Is this expected?

In your communication you were also mentioning an issue with the order in which particles are processed in the timing detectors. Is this addressed as well or not in this PR?

[fabiocos]

partly answering myself, in 2018 the geometry includes ZDC, and scenario D21 probed in the matrix touches FastTimerSD

[civanch]

@fabiocos , I agree with your comment on usage and validation of forward SD. The reason of this clean-up is to reduce code duplication, which make a problem for newcomer and provoke copy/paste of old code and propagation of old patterns from class to class. Forward SD classes , of course, do not affect CPU performance of situation but in this modification minor problems in hits are fixed, namely fastest particle is chosen for a hit, hit position and exit point of a hit is computed and filled correctly. The rest is pure technical - implementation of CMS code rules. This is the last general PR for developments for 10_2.

[fabiocos]

+1

[fabiocos]

for timing detectors for Phase2 we may have a closer dedicated check later on, in any case that code is still under potential development