Kleykamp additional truth changes #116

jdkio · 2024-05-31T03:58:10Z

Summary

Add additional truth info such as RecoTrackPrimaryParticleTrueTrackLength, which is true track length from reco track start to particle true end
Make fiducial volume config option
Add reco track true hit information - to compare with reco
True particle info - Now we save genie and geant particles. In the case of geant particles, we only save those that deposited energy inside the TMS. Genie particle information is saved regardless
Add Truth_Spill - saves info per spill instead of per slice. Useful since it only has truth info once instead of once per slice. And for pileup we'll need to decouple interaction-based truth info and slice-based truth info
Set reco track length to avg of U + V instead of CalculateTrackLength3d. See issue #114

Things still missing

Info from key points on outside edge of detectors. Only saving up to fiducial volume. Have IsInsideTMSMass
Associating true hit information with true particle information. Like to get more refined start/stop point in track. Reco track true hit information should be similar enough except in some edge cases. I suggest using true hit for start and then true particle end position for end.

Primary adjustments

Bugs:

Improvements:

Scripts:

…so add various vars related to whether interaction started in TMS, etc

…le info if primary or deposited energy in TMS

…ruth_changes

jdkio · 2024-05-31T14:44:07Z

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 · 2024-06-03T09:42:28Z

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?

…ng to memory hold

LiamOS · 2024-07-01T11:01:21Z

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 · 2024-07-01T18:04:05Z

Asa's been having some issues with comparisons between true and reco Z start and end positions, à la

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)

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 · 2024-07-04T00:24:10Z

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 · 2024-07-04T11:19:34Z

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 · 2024-07-05T16:32:07Z

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

scripts/Validation/Line_Candidates.h

AsaNehm · 2024-07-09T09:57:13Z

setup.sh

Hopefully this is not to be confused with the existing setup file for the new OS on the fermilab machines. Did someone check this in detail?

This was to make it backwards compatible with ProcessND.py, which expects setup.sh for when it's running on sl7. We should actually have setup.sh check its os and then run either os-specific script and then we'll have the best of both worlds

src/TMS_Constants.h

AsaNehm · 2024-07-09T10:02:38Z

src/TMS_Event.cpp


+  //std::cout<<"N total: "<<nTotal<<", N Primary: "<<nPrimary<<", N Interesting: "<<nInteresting<<", N charged: "<<nCharged<<", N high P: "<<nHighMomentum<<", N charged and low P: "<<nChargedAndLowMomentum<<", n TMS_TruePrimaryParticles: "<<TMS_TruePrimaryParticles.size()<<std::endl;


This should be removed if not necessary or transformed into a #ifdef debug ... #endif

AsaNehm · 2024-07-09T10:07:13Z

src/TMS_Geom.h

+    double GetTrackLength(std::vector<TVector3> nodes, bool ignore_y = false) {
+      //std::cout<<"Getting track length for "<<nodes.size()<<" nodes"<<std::endl;
+      double out = 0;
+      if (nodes.size() != 19) return out;


This is oddly specific. Why 19? Add comment at end of line?

AsaNehm · 2024-07-09T10:09:43Z

src/TMS_TrueParticle.cpp

  for (size_t i = 0; i < GetPositionPoints().size(); i++) {
-    double distance = abs(GetPositionPoints()[i].Z() - z);
-    if (distance <= max_z_dist) {
+    //std::cout << "pos: " << GetPositionPoints()[i].X() << ", "<< GetPositionPoints()[i].Y() << ", "<< GetPositionPoints()[i].Z() << "     " << GetPositionPoints()[i].Z() - prev_z << std::endl;


Take out or transform into #ifdef debug ... #endif

AsaNehm · 2024-07-09T10:13:25Z

src/TMS_TreeWriter.cpp

@@ -1074,7 +1202,7 @@ void TMS_TreeWriter::Fill(TMS_Event &event) {
    nHitsIn3DTrack[itTrack]         = (int) RecoTrack->Hits.size(); // Do we need to cast it? idk
 //    std::cout << "TreeWriter number of hits: " << nHitsIn3DTrack[itTrack] << std::endl;
    RecoTrackEnergyRange[itTrack]   =       RecoTrack->EnergyRange;
-    RecoTrackLength[itTrack]        =       RecoTrack->Length;
+    RecoTrackLength[itTrack]        =       0.5 * (TrackLengthU[itTrack] + TrackLengthV[itTrack]); // RecoTrack->Length;, 2d is better estimate than 3d because of y jumps


Maybe add a TODO for later after the Kalman filter works?

AsaNehm · 2024-07-09T10:13:54Z

src/TMS_TreeWriter.cpp

-    if (particle_info.energies.size() > 1) {
-      true_secondary_visible_energy = particle_info.energies[1];
-      true_secondary_particle_index = particle_info.indices[1];
+      //std::cout<<"checking for primary particle trackid"<<std::endl;


Is this necessary?

Which part? It's saving the secondary particle info. The cout can be deleted though

It's about the cout

AsaNehm · 2024-07-09T10:14:16Z

src/TMS_TreeWriter.cpp

@@ -1135,8 +1267,87 @@ void TMS_TreeWriter::Fill(TMS_Event &event) {
      if (itTrack < __TMS_MAX_LINES__) {
        setMomentum(RecoTrackPrimaryParticleTrueMomentumTrackStart[itTrack], tp.GetMomentumAtZ(start_z, max_z_distance));
        setPosition(RecoTrackPrimaryParticleTruePositionTrackStart[itTrack], tp.GetPositionAtZ(start_z, max_z_distance));
+        //std::cout << "Setting tp shite: " << tp.GetPositionAtZ(start_z, max_z_distance).X() << " " << tp.GetPositionAtZ(start_z, max_z_distance).Y() << " " << tp.GetPositionAtZ(start_z, max_z_distance).Z()  << ",\t" << start_z << " " <<  max_z_distance << std::endl;


Looks like @LiamOS's stash made it in based on the blame

LiamOS

Builds and runs fine from my checks, I've also been using some of this code on the Kalman branch so I trust it.

Approving now, will begin merging.

AsaNehm

Apart from some cosmetics I also approve

jdkio added 28 commits April 24, 2024 15:43

Fix make_hists to work with single files

1cc554a

Fix energy term of momentum vectors, at least particles in mass list

df9d5f0

Add track length function that uses vectors

ed55fa0

Fix confusion about geant4 vs genie particles

c1d2d4c

Add the concept of interesting particles to save info about

61c367a

Add all pdg masses with function

b820464

Add various true track length measures

39be02f

Change TMS fiducial to bars only

c9ca334

Make LAr fiducial a config option

9076cf1

Add Truth_Spill which has one entry per spill, instead of per slice

b77207c

Add index mapping to Truth_Info and reco trees

7586403

Fix neutrino X4 units and add to FillTruthFromGRooTracker

d4d0c96

Add fiducial as config

c1b8bae

Change functions to use config file fiducials

b1c79d8

Add inside TMS mass function, etc

b308caf

Fix X4 -> EvtVtx

ab49057

Add TMS_Spill, which has one entry per spill instead of per slice. Al…

d1a62ff

…so add various vars related to whether interaction started in TMS, etc

Add additional truth info about reco tracks

215872e

Add IsPrimary to TMS_TrueParticle

af81140

Add primary particles vs non-primary particles. Also save only partic…

cc88ed5

…le info if primary or deposited energy in TMS

Merge remote-tracking branch 'origin/main' into kleykamp_additional_t…

b0d381c

…ruth_changes

Add back setup.sh for SL7 users. Also adjust Makefiles

6913711

Fix typo in TMS_Geom

a481df3

Add all vars to clearing function

089e41a

Fix GetTrackLength bug by avoiding geometry bug

c72cbc5

Make default track length avg of U and V. Save reco track true hit info

bb291da

Add validation scripts

55e5ec2

Add LArSlicer.C

4e7f80d

jdkio requested a review from LiamOS May 31, 2024 03:58

jdkio marked this pull request as draft May 31, 2024 14:43

LiamOS requested a review from AsaNehm May 31, 2024 15:44

jdkio added 2 commits June 10, 2024 15:16

Fix issue with GetMaterials getting stuck with tiny step sizes, leadi…

ae6a3d1

…ng to memory hold

Fix TMS thick end to be past reco hits

6348b7d

stashing

58243b1

jdkio marked this pull request as ready for review July 4, 2024 00:15

AsaNehm reviewed Jul 9, 2024

View reviewed changes

scripts/Validation/Line_Candidates.h Outdated Show resolved Hide resolved

AsaNehm reviewed Jul 9, 2024

View reviewed changes

src/TMS_Constants.h Outdated Show resolved Hide resolved

AsaNehm reviewed Jul 9, 2024

View reviewed changes

LiamOS approved these changes Jul 11, 2024

View reviewed changes

Merge branch 'main' into kleykamp_additional_truth_changes

1744bb6

AsaNehm approved these changes Jul 11, 2024

View reviewed changes

LiamOS merged commit 0f4bb23 into main Jul 11, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kleykamp additional truth changes #116

Kleykamp additional truth changes #116

jdkio commented May 31, 2024

jdkio commented May 31, 2024

AsaNehm commented Jun 3, 2024 •

edited

Loading

LiamOS commented Jul 1, 2024

LiamOS commented Jul 1, 2024 •

edited

Loading

jdkio commented Jul 4, 2024 •

edited

Loading

LiamOS commented Jul 4, 2024

jdkio commented Jul 5, 2024

AsaNehm Jul 9, 2024

jdkio Jul 9, 2024

AsaNehm Jul 9, 2024

AsaNehm Jul 9, 2024

AsaNehm Jul 9, 2024

AsaNehm Jul 9, 2024

AsaNehm Jul 9, 2024

jdkio Jul 9, 2024

AsaNehm Jul 10, 2024

AsaNehm Jul 9, 2024

jdkio Jul 9, 2024

LiamOS left a comment

AsaNehm left a comment


		//std::cout<<"N total: "<<nTotal<<", N Primary: "<<nPrimary<<", N Interesting: "<<nInteresting<<", N charged: "<<nCharged<<", N high P: "<<nHighMomentum<<", N charged and low P: "<<nChargedAndLowMomentum<<", n TMS_TruePrimaryParticles: "<<TMS_TruePrimaryParticles.size()<<std::endl;

Kleykamp additional truth changes #116

Kleykamp additional truth changes #116

Conversation

jdkio commented May 31, 2024

Summary

Things still missing

Primary adjustments

Bugs:

Improvements:

Scripts:

jdkio commented May 31, 2024

AsaNehm commented Jun 3, 2024 • edited Loading

LiamOS commented Jul 1, 2024

LiamOS commented Jul 1, 2024 • edited Loading

jdkio commented Jul 4, 2024 • edited Loading

LiamOS commented Jul 4, 2024

jdkio commented Jul 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LiamOS left a comment

Choose a reason for hiding this comment

AsaNehm left a comment

Choose a reason for hiding this comment

AsaNehm commented Jun 3, 2024 •

edited

Loading

LiamOS commented Jul 1, 2024 •

edited

Loading

jdkio commented Jul 4, 2024 •

edited

Loading