MTD simulation: update Track ID stored in MTD SimHits #41318
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41318/35129
|
|
A new Pull Request was created by @fabiocos (Fabio Cossutti) for master. It involves the following packages:
@civanch, @Dr15Jones, @bsunanda, @makortel, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test workflow 20834.0 this needs to work also for D88 (i.e. old BTL geometry), as soon as it is supported |
|
@fabiocos , sorry for the mess - I prepared my PR during holiday, likely you did the same. As we discussed before about MC truth, there is no principal contradictions between these PRs, only technical. As far as I see two places:
What may be a problem (also we discussed before): I would prefer if SD classes fill TrackInformation container and not fill or create TrackWIthHistory objects. As we agreed, we should first discuss this approach with all involved parties and only after make a migration. The main reason - TrackInformation is design to store this information, we cannot store it in two places. The best approach is to extend TrackInformation if needed and not fill part of information there, another part - in TrackWithHistory with badly controlled relation between two. |
|
@civanch as you will see reviewing the code, I am not touching |
|
Pull request #41318 was updated. @civanch, @Dr15Jones, @bsunanda, @makortel, @mdhildreth, @AdrianoDee, @srimanob can you please check and sign again. |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e3abdb/31938/summary.html Comparison SummarySummary:
|
|
The output of the relval tests appears at least qualitatively consistent with expectations:
|
|
@civanch I have a presentation ready for next simulation meeting, which could be then linked to this PR as supplemental documentation. |
|
+1 |
cmsbuild
added
geometry-approved
simulation-approved
and removed
simulation-pending
geometry-pending
labels
| energyCut = m_p.getParameter<double>("EnergyThresholdForPersistencyInGeV") * CLHEP::GeV; //default must be 0.5 | ||
| energyHistoryCut = m_p.getParameter<double>("EnergyThresholdForHistoryInGeV") * CLHEP::GeV; //default must be 0.05 | ||
|
|
||
| setCuts(energyCut, energyHistoryCut); |
There was a problem hiding this comment.
this statement would treat tracks in BTL and in ETL in the same way, which is probably something we do not want. I suggest we possibly discuss this for a further development.
|
@srimanob, @AdrianoDee, please, sign this PR, we need it to be merged in order to continue activity on MC truth. |
|
+Upgrade From the upgrade-related package, the update on |
cmsbuild
added
fully-signed
upgrade-approved
and removed
pending-signatures
upgrade-pending
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) |
|
@perrotta ,@rappoccio, would it be possible to merge this PR? We have other PRs in mind, which should be done on top of it. |
|
+1 |
PR description:
This PR addresses two related issues, emerged in the past both in MTD studies and in discussions with HGCal experts (@rovere @felicepantaleo):
The latter is simply solved by instrumenting the
MtdSDclass with code using thecaloIDonSurfacemechanism implemented inTrackInformation.The former requires to implement a mechanism similar to that used for the tracker -> calo transition, with the additional complication that BTL is fully embedded in the tracker, and this can generate a bit more complex set of particles configurations.
In order to address this last issue, the
TrackInformationclass is instrumented with MTD-specific information, packed into a bitted word to avoid multipleboolvariables and save memory space. The information about transitions Tracker -> BTL and BTL -> Tracker are managed within theSteppingActionclass, while the handling of tracks created within the BTL volume is made in theNewTrackActionclass.The present PR is not supposed to modify the simulation history in terms of track / vertices stored in the event, or hit collections. Only the track ID stored within BTL/ETL hits can be affected. This PR is not completely addressing the problem of tracks created within the BTL volume, at present not stored in the event persistent history, and then entering the calo volume. The cut mechanism enabling the persistent storage is implemented in
MtdSD, but left switched off by default (using the standard cuts effectively preventing any action). The possible update of this part, affecting the event history, is deferred to a separate PR.This PR is in principle ineffective on run2/run3 simulation, but some checks in
SteppingActionandNewTrackActionare running also for that. Without an era-dependent flag, I do not see a way to avoid it.The update of existing MTD geometry regions for ProdCuts should be totally transparent, as they are so far used in one and only one place within CMSSW, and the existing behaviour is kept.
For further discussion, see https://indico.cern.ch/event/1275985/contributions/5359348/attachments/2628038/4545909/MTD-SIM_20230414.pdf
PR validation:
The code behaviour has been studies through detailed history and debugging printout on single muon, electron and pion guns, verifying the correctness of the logic for various cases: particle entering BTL, exiting BTL, loopers re-entering BTL, particles crossing ETL. A test on run2 simulation test wf 11607.0 do not show any difference, as expected.