SBNSoftware / sbncode

SBN Analysis Software
9 stars 27 forks source link

Single cryostat truth info #142

Open brucehoward-physics opened 3 years ago

brucehoward-physics commented 3 years ago

In FillTrueG4Particle in FillTrue.cxx of CAFMaker, it checks the volume where the particle is first in (cryostat_index) and then it gets the end point in this volume rather than overall end point.

So for tracks that start in one cryostat then go to the other, this creates complications (end point isn't the real end point).

I'm testing out an idea that would keep the start/end point in the initial volume and add in info from the second cryostat (if applicable, e.g. as it might be in ICARUS). Can update this thread when we've done more testing.

cjbacchus commented 3 years ago

I don't really know the details of how this would be used in analysis, but it seems like it would be more natural for the endpoint variable to store the true endpoint (potentially in the other cryostat), and add a new variable "endpoint in starting cryostat" (no good ideas for the name) that would presumably mostly duplicate that information, but be different in the case of cryostat-crossers.

brucehoward-physics commented 3 years ago

Yeah that's fair - it would mean the definition of "endpoint" changes when this feature would be picked up but maybe that's okay since it's current meaning is non-intuitive?

I think that would be only a minor additional logic on what I described above. What I described above is basically start/end in initial cryo as it is now, and a new scdy_start/scdy_end that is probably usually default but filled with what is in the secondary volume if something

This new idea would be be start in initial cryo, end in whichever cryo it is last in, and then maybe like prim_end and scdy_start if it's a cryo crosser, filled with default value if not crossing. So there would just need to be logic to decide if something goes in end or prim_end basically.

gputnam commented 3 years ago

In my opinion -- the "true" endpoint is often not very useful because G4 tracks particles well outside the detector volume, whereas with a track/shower/whatever you only get to see the particle through the detector.

For example, (start - end).Mag() tells me a useful quantity if "end" is the last point in the detector, but basically gibberish if "end" is some random point where G4 stopped tracking the particle.

So there is a good reason for the existing functionality. I agree with Bruce's point here that it doesn't handle the cross-cryostat case well, but I would prefer to augment the existing functionality than change it fundamentally.

cjbacchus commented 3 years ago

I think I meant "true location of the last time the particle exited a cryostat". That is the value you want to be comparing to the endpoint of any reconstructed track, if you are allowing track stitching between cryostats.

brucehoward-physics commented 3 years ago

That is also how I interpreted Chris's remark, hence "end in whichever cryo it is last in"

brucehoward-physics commented 3 years ago

I am going to test this re-formulation of the idea but it does lead to a slight knock-on question -- the true particle quantities such as:

Are currently also defined by the initial cryostat.

Crosses_tpc and cont_tpc should remain unchanged I think, as they are all really about the initial tpc. But "contained" could take on two meanings. Probably the simplest meaning is to leave it referring to containment in the initial cryostat or if it escapes that cryostat. However, one could also ask if it is contained in either cryostat. Length and wallout can have similarly dueling meanings.

brucehoward-physics commented 3 years ago

The idea would be something like here: https://github.com/SBNSoftware/sbncode/commit/d83bb7b69b3b883b18db88b1186460952ffda043

wesketchum commented 2 years ago

@brucehoward-physics still something needed? Already addressed otherwise? Either an update here (and if it's still needed on timescale of 2021C) or close out ...

brucehoward-physics commented 2 years ago

I think @gputnam had a recent update that was accepted (reviewed by Chris I think?) and included splitting out the truth info based on cryostat. I haven't tried to use it (yet).

brucehoward-physics commented 2 years ago

Can you confirm Gray?

FernandaPsihas commented 2 years ago

@grayputnam @brucehoward-physics are we good on this issue or does something still need to be done?

brucehoward-physics commented 2 years ago

Oh sorry we had some chat on a Slack convo the other day and not here.

Per Gray: "I recently merged a PR into sbncode which partially addressed this issue"

But I haven't properly looked yet at this PR/the new sbnana to see if it fully addressed this issue or if there is something remaining related to this issue. Will update here when I have looked.

brucehoward-physics commented 2 years ago

Okay had a look at Gray's PR and chatted with him briefly over Slack (thanks!). I am recommending this issue remain open.

The changes didn't impact the "FillTrueG4Particle" info so for example if a muon crossed from one cryostat to the other in ICARUS we wouldn't have access to its stop point in the other cryostat. (Or possibly for other similar things of interest, like if it exits the other cryostat and so is uncontained even considering the other cryostat.)

These can be useful, if for example you are trying to stitch tracks between the CryoE reco and CryoW reco. So I recommend this issue remain open.

ibsafa commented 1 month ago

@brucehoward-physics is this still unresolved?