Geant4 sensitive detectors clean-up 6 #23480

civanch · 2018-06-05T10:55:58Z

This PR includes clean-ups for muon Geant4 sensitive detectors and additional modifications in the base class, tracker and calo sensitive detectors:

TrackInformation object access is implemented in the base class and it is used in Calo, Tracker, Muon derived classes (removed multiple implementations of the same code)
Fixed dependences between these packages - removed circular dependence between SimG4CMS/SimMuon and SimG4Core/Application, added few really needed dependencies
Unified transformation from Geant4 space coordinates to CMS coordinates in muon and and tracker
Use "#include cstdint" instead of boost
In MuonSensitiveDetector there were computations needed only for debug printout, these computations are protected by #ifdef
Use LogVerbatim and LogDebug where possible, removed abort() and cerr, use cms exceptions instead
Use const pointers to Geant4 objects
clean up Observer interface - make update() methods protected
removed unnecessary methods, header, unreasonable comments, old commented lines and add new comments

Correctness of this PR means no change of the results.

cmsbuild · 2018-06-05T10:56:17Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-06-05T10:58:15Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23480/5028

cmsbuild · 2018-06-05T10:58:32Z

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

It involves the following packages:

SimG4CMS/Calo
SimG4CMS/Muon
SimG4CMS/Tracker
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

civanch · 2018-06-05T11:02:22Z

please test

cmsbuild · 2018-06-05T11:02:40Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28472/console Started: 2018/06/05 13:03

cmsbuild · 2018-06-05T11:12:12Z

-1

Tested at: 559a846

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

I found follow errors while testing this PR

Failed tests: Build

Build:

I found an error when building:

tmp/slc6_amd64_gcc630/src/SimG4CMS/Muon/src/SimG4CMSMuon/MuonGEMFrameRotation.cc.o:(.data.rel.ro._ZTI20MuonGEMFrameRotation[_ZTI20MuonGEMFrameRotation]+0x10): undefined reference to `typeinfo for MuonFrameRotation'
tmp/slc6_amd64_gcc630/src/SimG4CMS/Muon/src/SimG4CMSMuon/MuonSensitiveDetector.cc.o: In function `MuonSensitiveDetector::MuonSensitiveDetector(std::__cxx11::basic_string, std::allocator > const&, DDCompactView const&, SensitiveDetectorCatalog const&, edm::ParameterSet const&, SimTrackManager const*)':
MuonSensitiveDetector.cc:(.text+0x29cd): undefined reference to `vtable for MuonFrameRotation'
tmp/slc6_amd64_gcc630/src/SimG4CMS/Muon/src/SimG4CMSMuon/MuonEndcapFrameRotation.cc.o:(.data.rel.ro._ZTI23MuonEndcapFrameRotation[_ZTI23MuonEndcapFrameRotation]+0x10): undefined reference to `typeinfo for MuonFrameRotation'
tmp/slc6_amd64_gcc630/src/SimG4CMS/Muon/src/SimG4CMSMuon/MuonME0FrameRotation.cc.o:(.data.rel.ro._ZTI20MuonME0FrameRotation[_ZTI20MuonME0FrameRotation]+0x10): undefined reference to `typeinfo for MuonFrameRotation'
collect2: error: ld returned 1 exit status
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_2_X_2018-06-04-2300/src/SimG4CMS/ShowerLibraryProducer/plugins/module.cc 
gmake: *** [tmp/slc6_amd64_gcc630/src/SimG4CMS/Muon/src/SimG4CMSMuon/libSimG4CMSMuon.so] Error 1
Leaving library rule at SimG4CMS/Muon
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_2_X_2018-06-04-2300/src/SimG4CMS/ShowerLibraryProducer/plugins/HcalForwardLibWriter.cc 
>> Building big object file tmp/slc6_amd64_gcc630/src/SimG4CMS/Forward/plugins/SimG4CMSForwardPlugins/bigobj/SimG4CMSForwardPlugins.obj

cmsbuild · 2018-06-05T11:12:14Z

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

cmsbuild · 2018-06-07T09:06:04Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-06-07T09:07:41Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23480/5087

cmsbuild · 2018-06-07T09:08:02Z

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

civanch · 2018-06-07T09:19:51Z

please test

cmsbuild · 2018-06-07T09:20:12Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28534/console Started: 2018/06/07 11:20

cmsbuild · 2018-06-07T12:01:56Z

+1
Tested at: 9b8c43d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23480/28534/summary.html

cmsbuild · 2018-06-07T12:01:59Z

Comparison job queued.

cmsbuild · 2018-06-07T14:08:32Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23480/28534/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: 2902639
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2902447
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 · 2018-06-07T14:10:30Z

+1

cmsbuild · 2018-06-07T14:10:52Z

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)

civanch · 2018-06-07T14:12:55Z

@fabiocos , this PR now provides identical results versus baseline. Can you , please, check and merge it. It includes bug-fix of circular dependence of SIM packages, the effect of which are unclear to me.

fabiocos · 2018-06-08T09:10:10Z

SimG4CMS/Muon/src/MuonSensitiveDetector.cc

    }
  }
-  return false;
+  return true;


@civanch for my understanding, what is implied by this change of logic? What was the triggered action, if any, by a "false" value returned?

There is no difference to me - the returned bool does not used anywhere. I put "true" in order not make impression to newcomer, that a choice should be done between "true" and "false".

fabiocos · 2018-06-08T09:11:03Z

SimG4CMS/Muon/interface/MuonSensitiveDetector.h

 class SimTrackManager;

 class MuonSensitiveDetector : 
 public SensitiveTkDetector,
-public Observer<const BeginOfEvent*>,
-public Observer<const EndOfEvent*>


@civanch there is no action in the EndOfEvent, this is the reason to remove this line, am I correct?

Yes, if methods will not be empty, they may be returned back.

@fabiocos , one of the goals of this clean-up is to take out methods/code which doing nothing but is copy/paste from one SD class to another.

fabiocos · 2018-06-08T09:12:14Z

SimG4CMS/Muon/src/MuonSensitiveDetector.cc


 #include "FWCore/MessageLogger/interface/MessageLogger.h"

 #include <iostream>

+//#define DebugLog


@civanch I understand that you keep the revision of the verbosity that we discussed for a further PR, am I correct?

In this code it turned out that a significant part of computation is done only for debugging. It much less work is to include #ifdef, LogDebug itself cannot serving for that.

@fabiocos , in this PR I tried improving verbosity using Verbatim stream and LogDebug where possible but in two places seems the best solution is to add #ifdef

@civanch I was referring to the standardisation to EDM_ML_DEBUG...

fabiocos · 2018-06-08T18:08:52Z

+1

civanch added 2 commits June 5, 2018 12:36

cleanup muons SD

941c7bf

more cleanup muons SD

559a846

cmsbuild added this to the CMSSW_10_2_X milestone Jun 5, 2018

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures simulation-pending tests-pending labels Jun 5, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 5, 2018

cmsbuild added tests-started and removed tests-pending labels Jun 5, 2018

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Jun 5, 2018

added forgotten

5f1275d

cmsbuild added code-checks-pending comparison-pending and removed code-checks-approved comparison-notrun tests-rejected labels Jun 5, 2018

cmsbuild added comparison-pending tests-pending labels Jun 7, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 7, 2018

cmsbuild added tests-started and removed tests-pending labels Jun 7, 2018

cmsbuild added tests-approved and removed tests-started labels Jun 7, 2018

cmsbuild added comparison-available and removed comparison-pending labels Jun 7, 2018

cmsbuild added fully-signed simulation-approved and removed pending-signatures simulation-pending labels Jun 7, 2018

fabiocos reviewed Jun 8, 2018

View reviewed changes

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

cmsbuild merged commit 16237f9 into cms-sw:master Jun 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Geant4 sensitive detectors clean-up 6 #23480

Geant4 sensitive detectors clean-up 6 #23480

civanch commented Jun 5, 2018 •

edited

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

civanch commented Jun 5, 2018

cmsbuild commented Jun 5, 2018 •

edited

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

civanch commented Jun 7, 2018

cmsbuild commented Jun 7, 2018 •

edited

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

civanch commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

civanch commented Jun 7, 2018

fabiocos Jun 8, 2018

civanch Jun 8, 2018

fabiocos Jun 8, 2018

civanch Jun 8, 2018

civanch Jun 8, 2018

fabiocos Jun 8, 2018

civanch Jun 8, 2018

civanch Jun 8, 2018

fabiocos Jun 8, 2018

fabiocos commented Jun 8, 2018

Geant4 sensitive detectors clean-up 6 #23480

Geant4 sensitive detectors clean-up 6 #23480

Conversation

civanch commented Jun 5, 2018 • edited

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

civanch commented Jun 5, 2018

cmsbuild commented Jun 5, 2018 • edited

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 5, 2018

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

civanch commented Jun 7, 2018

cmsbuild commented Jun 7, 2018 • edited

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

civanch commented Jun 7, 2018

cmsbuild commented Jun 7, 2018

civanch commented Jun 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiocos commented Jun 8, 2018

civanch commented Jun 5, 2018 •

edited

cmsbuild commented Jun 5, 2018 •

edited

cmsbuild commented Jun 7, 2018 •

edited