search

Mu2e / Offline

Offline software for the Mu2e experiment
Apache License 2.0
8 stars 82 forks source link

Phasing out StepPointMC #383

Open gaponenko opened 3 years ago

gaponenko commented 3 years ago

Copying a comment from a discussion here https://github.com/Mu2e/Offline/pull/379#issuecomment-803443585

I think StepPointMC is a bit of a mess because it is trying to cover too many use cases. It tries to be both a "point", which ideally should be just a record of particle state at that point (position, momentum, time, proper time, and a ptr to its SimParticle) and nothing else. Such "points" can be naturally produced by our infinitesimally thin virtual detectors, and are useful for analysis. But then there is also the "step", which has things like various kinds of energy deposition, step length, etc. The "step" information is used by digitization, but different detectors need different infos. For example visibleEnergyDeposit is of no interest to the tracker. It would be better if each detector produced their own "steps" without going through StepPointMC, like ExtMon does (see MCDataProducts/inc/ExtMonFNALSimHit.hh ) Then we could have a lean and clean "analysis" object, and a set of detector-defined steps, without a shared class that has to satisfy everyone.

gaponenko commented 3 years ago

One thing that makes the transition easier is that it can be done piecewise instead of globally, migrating one detector at a time away from StepPointMC.

kutschke commented 3 years ago

Adding a comment from Dave that was on a different thread - one obvious need is for a specialized class for steps in VDs.

resnegfk commented 3 years ago

Thanks for accepting #379 PR.

I agree that the object combines hit and step information, but they are connected by the spacial info, so it may not be so bad, but probably could be optimized.

Regarding it usefulness, now that the post momentum is there, it would be more correct/efficient to restore the particle state in the multistage jobs from the post step info, as that step had been made already, instead of repeating it in some sense (likely with a different outcome).

Regarding CrvSteps and similar use cases, care needs to be taken to check what happened in the post step phase, to make sure the momentum is not e.g. zero and compare the velocity prediction with what is in the post step.

Krzysztof

brownd1978 commented 1 year ago

I would like to create a 'DetectorStep' class to summarize StepPointMCs for passive materials (IPA, target foils, etc). This would avoid downstream users having to add multiple StepPointMC objects to get physically meaningful results, and further insulates us from G4 details. This will eliminate StepPointMCs from the dts and further downstream output. They will still be used in resampling; perhaps a dedicated class there makes sense to.

gaponenko commented 1 year ago

Hi Dave,

"detector" step sounds too generic. ExtMon already uses a pixel specific step class. Needs of different detectors are different. Perhaps for the intended use it can be "PassiveMaterialStep"?

Andrei

From: David Brown @.***> Sent: Tuesday, September 19, 2023 11:52 AM To: Mu2e/Offline Cc: Andrei Gaponenko; Author Subject: Re: [Mu2e/Offline] Phasing out StepPointMC (#383)

I would like to create a 'DetectorStep' class to summarize StepPointMCs for passive materials (IPA, target foils, etc). This would avoid downstream users having to add multiple StepPointMC objects to get physically meaningful results, and further insulates us from G4 details. This will eliminate StepPointMCs from the dts and further downstream output. They will still be used in resampling; perhaps a dedicated class there makes sense to.

— Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Mu2e_Offline_issues_383-23issuecomment-2D1726078214&d=DwMCaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=O47fc5vzDTR2V_gla4Ub0Q&m=_WYQ9pBrGNZD2oF0YnV7W3-3DC8vgNRcfyfQSTt4lWGseNnpPtc3Yo9RI-UHY9sh&s=vIQz0YMfnS8GVBv636pA13suZZ6sxyJKvsU_uafF_Zs&e=, or unsubscribehttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAXVCGQ45VRVPTAIL46WXXDX3HEWVANCNFSM4ZQ3WUHA&d=DwMCaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=O47fc5vzDTR2V_gla4Ub0Q&m=_WYQ9pBrGNZD2oF0YnV7W3-3DC8vgNRcfyfQSTt4lWGseNnpPtc3Yo9RI-UHY9sh&s=gha6ixI_jKGMfPbEGzfE_Hp74D2PBqP2AWmzSTqwWo8&e=. You are receiving this because you authored the thread.Message ID: @.***>

brownd1978 commented 1 year ago

On Tue, Sep 19, 2023 at 10:01 AM Andrei Gaponenko @.***> wrote:

Hi Dave,

"detector" step sounds too generic. ExtMon already uses a pixel specific step class. Needs of different detectors are different. Perhaps for the intended use it can be "PassiveMaterialStep"?

Yes, like that. 'DetectorStep' is the general term for StrawGasStep, CrvStep, and CaloCrystalStep.

Andrei

From: David Brown @.***> Sent: Tuesday, September 19, 2023 11:52 AM To: Mu2e/Offline Cc: Andrei Gaponenko; Author Subject: Re: [Mu2e/Offline] Phasing out StepPointMC (#383)

I would like to create a 'DetectorStep' class to summarize StepPointMCs for passive materials (IPA, target foils, etc). This would avoid downstream users having to add multiple StepPointMC objects to get physically meaningful results, and further insulates us from G4 details. This will eliminate StepPointMCs from the dts and further downstream output. They will still be used in resampling; perhaps a dedicated class there makes sense to.

— Reply to this email directly, view it on GitHub< https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Mu2e_Offline_issues_383-23issuecomment-2D1726078214&d=DwMCaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=O47fc5vzDTR2V_gla4Ub0Q&m=_WYQ9pBrGNZD2oF0YnV7W3-3DC8vgNRcfyfQSTt4lWGseNnpPtc3Yo9RI-UHY9sh&s=vIQz0YMfnS8GVBv636pA13suZZ6sxyJKvsU_uafF_Zs&e=>, or unsubscribe< https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAXVCGQ45VRVPTAIL46WXXDX3HEWVANCNFSM4ZQ3WUHA&d=DwMCaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=O47fc5vzDTR2V_gla4Ub0Q&m=_WYQ9pBrGNZD2oF0YnV7W3-3DC8vgNRcfyfQSTt4lWGseNnpPtc3Yo9RI-UHY9sh&s=gha6ixI_jKGMfPbEGzfE_Hp74D2PBqP2AWmzSTqwWo8&e=>.

You are receiving this because you authored the thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/383#issuecomment-1726093765, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH572B7JWPMRXSM6OWAILX3HFU3ANCNFSM4ZQ3WUHA . You are receiving this because you commented.Message ID: @.***>

-- David Brown @.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

kutschke commented 1 year ago

I like the name PasssiveMaterialStep.

I believe the plan is to produce StepPointMCs as before but to collect them into PasssiveMaterialStep objects at the same time as other DetectorStep objects are made. is that right?

If so, I recommend we go ahead.

brownd1978 commented 1 year ago

On Tue, Sep 19, 2023 at 11:28 AM Rob Kutschke @.***> wrote:

I like the name PasssiveMaterialStep.

I believe the plan is to produce StepPointMCs as before but to collect them into PasssiveMaterialStep objects at the same time as other DetectorStep objects are made. is that right?

If so, I recommend we go ahead.

Yes. Mostly this can be coded by stripping out bits of MakeStrawGasSteps, etc. It's on my list, but anyone else who wants it is welcome to take it! Dave

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/383#issuecomment-1726273602, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH57ZY2HEFGDWJXCDYV7TX3HP6HANCNFSM4ZQ3WUHA . You are receiving this because you commented.Message ID: @.***>

-- David Brown @.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720