DUNE / dune-tms

DUNE ND Temporary Muon Spectrometer
0 stars 1 forks source link

Kleykamp additional truth changes #116

Closed jdkio closed 2 months ago

jdkio commented 4 months ago

Summary

Things still missing

Primary adjustments

Bugs:

Improvements:

Scripts:

jdkio commented 4 months ago

Ran larger sample on grid and many jobs got held due to memory limits. Either from saving all particles, or there's a memory leak. Investigating

AsaNehm commented 4 months ago

For the averaging of TrackLengthU and TrackLengthV you mean the reco tracks or the truth ones? For the reco this shouldn't be necessary, as that is basically already done by using RecoX instead of the x position of the hits which averages the position of hit pairs when calculating the track length TrackLengthU and TrackLengthV would then also need to be extended to TrackLengthX to account for X layers as well which would get difficult for only very few X layers So I strongly oppose this averaging Edit: Ah sorry, you're averaging for the y position. This zig-zagging is a problem that indeed could and most likely would be solved with averaging. Only problem is still for geometries with X layers On the other hand this most likely wouldn't solve the track length truth vs. reco problem entirely, as as far as I understand makes the zig-zagging the 'track' longer and should therefore have a higher density-weighted track length than the truth. But the opposite was the case so far. Would that be only the effect from the different truth track start?

LiamOS commented 3 months ago

At last Friday's meeting we discussed merging this as the new variables seem to be (at least mostly) working correctly. Is this PR still up to date and nominally ready to merge?

LiamOS commented 3 months ago

Asa's been having some issues with comparisons between true and reco Z start and end positions, à la RecoStartZ while the X and Y distributions appear fine.

I dug into this today and it seems to come from the interactions between GetPositionAtZ() and GetPositionPoints() in TMS_TrueParticle.cpp. The positions points get within some mm/cm of the desired z value, but do not get arbitrarily close. The closest point seems to be returned, meaning the returned Z and the given Z are not the same.

This can be """fixed""" by keeping track of the signed distance from the returned point and the given Y, and adding this back onto the returned point: (sorry for image diff) gitdiff

To do this more correctly I'd check the momentum at that Z, and then add in a distance*mom[0/1]/mom[2] component to correct X and Y to the desired point.

jdkio commented 3 months ago

Unfortunately there are no good answers. The idea behind the variable was to measure the start and stop points of the particle. The best solution is probably to find the exact point it enters the front of the TMS and make that the point you save, not base it on the z of the reco track. Otherwise you're always within 1 plane of the right answer.

So we'd probably want to update the function to look for start and end points inside the TMS. If it can't find those, then find the point that we enter/exit the TMS, interpolating from the last point outside and the first point inside (and vice versa for exiting particles). So it's basically what you said, but based on the fiducial volume z (and x/y for side-entering particles) instead of reco z.

And then there's the issue of reco tracks that are made from a particle that scatters and makes a continuing track. While technically two particles, we can treat that as a single track for the purposes of energy reconstruction. But currently we'd get the wrong starting or ending position for the track. Maybe our studies should simply add a cut to remove those for now, instead of trying to fix our definition. One could detect them by looking at the primary particle responsible for the front/back of a reco track and noticing that they're different. For now, we could try removing any reco track that has a substantial amount of energy from a secondary particle

I don't see myself having any cycles to finish this in the near future so if you can pick up the mantle, that would be greatly appreciated!

I also added RecoTrackTrueHitPosition, which gives the true hits of the reco track. This at least lets you check the x and y positions, but it does have that issue of always being right within 1 plane of z since it's the same z position by definition

LiamOS commented 3 months ago

Thanks for the detailed reply. I'm fairly sure I understand what's going on for these variables now (and know mostly where we're failing).

For tweaking the variables to match the truth closer, this wasn't a significant issue in X and Y (for physics/geometry reasons) and the Z can be easily fixed. If @AsaNehm needs more than the Z fix it likely won't be until after the PDR.

One thing I did want to bring up for discussion was whether a generic RecoTrackTrueVar should live in the Reco_Tree or in Truth_Info. My T2K brain says they should go in the Reco_Tree, as these are still reconstruction dependent quantities to some degree.

jdkio commented 3 months ago

Well they do have truth info, so that's why they're in truth_info. When we have real data, the truth tree can be turned off. If we have it in the reco tree, then we'd need to turn off those specific branches. And it increases the chance that someone might use them as reco quantities, not knowing that it's truth info. I think that's why we originally had the two separate trees