Geant4 sensitive detector cleanup-2 #21739

civanch · 2017-12-17T18:26:15Z

Continue clean-up of Geant4 sensitive detectors. Now only classes related to Tracker are modified:

use const pointers and reference to Geant4 objects
use const reference for CMSSW objects
abort() replaced by cms::Exception
compute values, which are used for printout and for debug only, if this mode enabled
removed checks that point is inside sensitive detector - this condition should be true by construction
use types more consistently
removed unused classes, headers and methods
In this classes were no problems, so this clean-up should not change any result.

cmsbuild · 2017-12-17T18:26:57Z

The code-checks are being triggered in jenkins.

civanch · 2017-12-17T18:27:18Z

please test

cmsbuild · 2017-12-17T18:29:24Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21739/2636

cmsbuild · 2017-12-17T18:30:12Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25147/console Started: 2017/12/17 23:46

cmsbuild · 2017-12-17T18:30:13Z

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

It involves the following packages:

SimG4CMS/Tracker
SimG4Core/Application
SimG4Core/Physics
SimG4Core/SensitiveDetector

@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

VinInn · 2017-12-17T19:10:05Z

SimG4CMS/Tracker/interface/TrackerG4SimHitNumberingScheme.h

 private:
-    TouchableToHistory * ts;
+  TouchableToHistory * ts;


what about unique_ptr or even just by holding it value?
the copy behavior of this class shall be better constrained...
btw: what is the real difference between a TrackerG4SimHitNumberingScheme and a TouchableToHistory ??

VinInn · 2017-12-17T19:11:57Z

SimG4CMS/Tracker/interface/TkAccumulatingSensitiveDetector.h

-    G4TrackToParticleID * myG4TrackToParticleID;
-    TrackInformation* getOrCreateTrackInformation(const G4Track *);
+    double rTracker2;   // tracker volume R^2
+    double rTracker;    // tracker volume R


were float, now double. why?

it is compared with double, not part of hit, so saving of memory is not important.

VinInn · 2017-12-17T19:13:08Z

please consider to make classes "final" is it applies

VinInn · 2017-12-17T19:15:30Z

SimG4CMS/Tracker/src/TrackerFrameRotation.cc

@@ -1,6 +1,6 @@
 #include "SimG4CMS/Tracker/interface/TrackerFrameRotation.h"
 #include "G4SystemOfUnits.hh"

-Local3DPoint TrackerFrameRotation::transformPoint(Local3DPoint & point,G4VPhysicalVolume * v=nullptr) const {
+Local3DPoint TrackerFrameRotation::transformPoint(const Local3DPoint & point,const G4VPhysicalVolume*) const {
  return Local3DPoint(point.x()/cm,point.y()/cm,point.z()/cm);


we should find a way to avoid all these "useless" divisions

cmsbuild · 2017-12-18T00:33:46Z

+1
Tested at: 5841fd0
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21739/25147/summary.html

cmsbuild · 2017-12-18T00:33:49Z

Comparison job queued.

cmsbuild · 2017-12-18T04:48:42Z

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/build/cmsbld/jenkins-workarea/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-21739/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 16207 differences found in the comparisons
DQMHistoTests: Total files compared: 27
DQMHistoTests: Total histograms compared: 2836825
DQMHistoTests: Total failures: 52525
DQMHistoTests: Total nulls: 2
DQMHistoTests: Total successes: 2784120
DQMHistoTests: Total skipped: 178
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.790000000175 KiB( 23 files compared)
Checked 113 log files, 9 edm output root files, 27 DQM output files

civanch · 2017-12-18T08:39:26Z

history is changed - needs to be understood.

cmsbuild · 2017-12-20T01:23:08Z

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. 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)

cmsbuild · 2017-12-20T01:26:16Z

+1
Tested at: 461797f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21739/25190/summary.html

cmsbuild · 2017-12-20T01:26:18Z

Comparison job queued.

cmsbuild · 2017-12-20T05:00:03Z

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/build/cmsbld/jenkins-workarea/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-21739/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 24 differences found in the comparisons
DQMHistoTests: Total files compared: 27
DQMHistoTests: Total histograms compared: 2831371
DQMHistoTests: Total failures: 1942
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2829251
DQMHistoTests: Total skipped: 178
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 1.61000000007 KiB( 23 files compared)
Checked 113 log files, 9 edm output root files, 27 DQM output files

civanch · 2017-12-20T07:56:13Z

@davidlange6 , all comments are implemented, differences all are in 2018 WF and are not connected with this PR.

davidlange6 · 2018-01-02T20:12:38Z

please test

cmsbuild · 2018-01-02T20:14:31Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25298/console Started: 2018/01/02 21:15

cmsbuild · 2018-01-02T22:15:43Z

+1
Tested at: 461797f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21739/25298/summary.html

cmsbuild · 2018-01-02T22:15:46Z

Comparison job queued.

cmsbuild · 2018-01-02T23:34:46Z

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/build/cmsbld/jenkins-workarea/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-21739/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 26
DQMHistoTests: Total histograms compared: 2775243
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2775073
DQMHistoTests: Total skipped: 169
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 1.25999999975 KiB( 21 files compared)
Checked 110 log files, 9 edm output root files, 26 DQM output files

davidlange6 · 2018-01-08T16:42:29Z

merge

civanch added 2 commits December 17, 2017 19:15

cleanup sensitive tracker

b44c2ce

cleanup sensitive tracker

5841fd0

cmsbuild added this to the CMSSW_10_0_X milestone Dec 17, 2017

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures simulation-pending tests-pending labels Dec 17, 2017

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 17, 2017

cmsbuild added tests-started and removed tests-pending labels Dec 17, 2017

VinInn reviewed Dec 17, 2017

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Dec 18, 2017

cmsbuild added comparison-available and removed comparison-pending labels Dec 18, 2017

cmsbuild added the simulation-approved label Dec 20, 2017

cmsbuild added tests-approved and removed tests-started labels Dec 20, 2017

cmsbuild added comparison-available and removed comparison-pending labels Dec 20, 2017

davidlange6 mentioned this pull request Dec 20, 2017

Tracking open items for 1000pre3 #21749

Closed

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels Jan 2, 2018

cmsbuild added tests-started and removed tests-pending labels Jan 2, 2018

cmsbuild added tests-approved and removed tests-started labels Jan 2, 2018

cmsbuild added comparison-available and removed comparison-pending labels Jan 2, 2018

cmsbuild added orp-approved and removed orp-pending labels Jan 8, 2018

cmsbuild merged commit 6751577 into cms-sw:master Jan 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Geant4 sensitive detector cleanup-2 #21739

Geant4 sensitive detector cleanup-2 #21739

civanch commented Dec 17, 2017 •

edited

cmsbuild commented Dec 17, 2017

civanch commented Dec 17, 2017

cmsbuild commented Dec 17, 2017

cmsbuild commented Dec 17, 2017 •

edited

cmsbuild commented Dec 17, 2017

VinInn Dec 17, 2017

VinInn Dec 17, 2017

civanch Dec 18, 2017

VinInn commented Dec 17, 2017

VinInn Dec 17, 2017

cmsbuild commented Dec 18, 2017

cmsbuild commented Dec 18, 2017

cmsbuild commented Dec 18, 2017

civanch commented Dec 18, 2017

cmsbuild commented Dec 20, 2017

cmsbuild commented Dec 20, 2017

cmsbuild commented Dec 20, 2017

cmsbuild commented Dec 20, 2017

civanch commented Dec 20, 2017 •

edited

davidlange6 commented Jan 2, 2018

cmsbuild commented Jan 2, 2018 •

edited

cmsbuild commented Jan 2, 2018

cmsbuild commented Jan 2, 2018

cmsbuild commented Jan 2, 2018

davidlange6 commented Jan 8, 2018

Geant4 sensitive detector cleanup-2 #21739

Geant4 sensitive detector cleanup-2 #21739

Conversation

civanch commented Dec 17, 2017 • edited

cmsbuild commented Dec 17, 2017

civanch commented Dec 17, 2017

cmsbuild commented Dec 17, 2017

cmsbuild commented Dec 17, 2017 • edited

cmsbuild commented Dec 17, 2017

VinInn Dec 17, 2017

Choose a reason for hiding this comment

VinInn Dec 17, 2017

Choose a reason for hiding this comment

civanch Dec 18, 2017

Choose a reason for hiding this comment

VinInn commented Dec 17, 2017

VinInn Dec 17, 2017

Choose a reason for hiding this comment

cmsbuild commented Dec 18, 2017

cmsbuild commented Dec 18, 2017

cmsbuild commented Dec 18, 2017

civanch commented Dec 18, 2017

cmsbuild commented Dec 20, 2017

cmsbuild commented Dec 20, 2017

cmsbuild commented Dec 20, 2017

cmsbuild commented Dec 20, 2017

civanch commented Dec 20, 2017 • edited

davidlange6 commented Jan 2, 2018

cmsbuild commented Jan 2, 2018 • edited

cmsbuild commented Jan 2, 2018

cmsbuild commented Jan 2, 2018

cmsbuild commented Jan 2, 2018

davidlange6 commented Jan 8, 2018

civanch commented Dec 17, 2017 •

edited

cmsbuild commented Dec 17, 2017 •

edited

civanch commented Dec 20, 2017 •

edited

cmsbuild commented Jan 2, 2018 •

edited