Different Geant4 SteppingActions for Run3 and Phase2 #41404
Conversation
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41404/35270
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41404/35271
|
|
A new Pull Request was created by @civanch (Vladimir Ivantchenko) for master. It involves the following packages:
@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8f8981/32140/summary.html Comparison SummarySummary:
|
|
+1 |
cmsbuild
added
simulation-approved
fully-signed
and removed
simulation-pending
pending-signatures
labels
|
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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
| @@ -77,13 +77,17 @@ class RunManagerMT { | |||
| // need a non-const pointer. Thread-safety is handled inside Geant4. | |||
| inline PhysicsList* physicsListForWorker() const { return m_physicsList.get(); } | |||
|
|
|||
| inline bool isPhase2() const { return m_isPhase2; } | |||
There was a problem hiding this comment.
I think that these new methods/variables which include Phase2 in the name should be better renamed as (e.g.) isHgcalMtd() , m_isHgcalMtd , etc. because they do not refere to the Phase2 detector in general, but only to those two (three: HGCAL, ETL, BTL) subdetectors. Otherwise, named as they are now it can become misleading
There was a problem hiding this comment.
@perrotta , it is easy to implement, what you propose, but the choice if it is Phase2 or not in this PR means a choice of SteppingAction class, which is used for simulation of all sub-detectors. I would expect that in future some other parameters/classes will be chosen using this flag. Check on geometry does not introduce an uncertainty. For today I do not see any problem with it.
As an alternative, instead of checking on geometry this flag may be defined in configuration file g4SimHits by hand with modifiers....
There was a problem hiding this comment.
Ok, thank you @civanch
Then I misunderstood: I thought that the choice of the SteppingAction was specific for every subdetector. If it is not so, then the name that you chose is correct
| @@ -340,6 +342,7 @@ void RunManagerMTWorker::initializeG4(RunManagerMT* runManagerMaster, const edm: | |||
|
|
|||
| // Set the physics list for the worker, share from master | |||
| PhysicsList* physicsList = runManagerMaster->physicsListForWorker(); | |||
| m_isPhase2 = runManagerMaster->isPhase2(); | |||
There was a problem hiding this comment.
There is probably no need to make this variable a class member of RunManagerMTWorker, it could probably better stay local. On the other hand, it is done as such also or the other m_* boolean class member, and perhaps there was a good reason to do so...
|
+1 |
PR description:
Recent development for Phase-2 MC truth handling (https://indico.cern.ch/event/1275985/) demonstrate that Geant4 SteppingAction logic in the case of Run2 or Run3 is slightly different from Phase2, because of existence of MTD and HGCal new detectors. This code is called at each simulation step for each track, in this PR we introduce usage of different classes for these two cases. No change of results in any WF is expected.
PR validation:
private
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for: NO