Feature/3D Kalman Filter #122

LiamOS · 2024-07-11T13:18:29Z

Have merged current main into this branch just now.

Current behaviour:

Assumes p=0 at track end, propagates backwards only
All reconstructed positions saved in the TMS_Track obj
Returns single value for momentum (need to check this is reasonable)
Can be switched on in config
Seems to just keep running if something terrible happens (good thing?)

Please give a scan over the Reco and TreeWriter files to make sure I haven't destroyed anything unintentionally.

AsaNehm · 2024-07-11T13:40:56Z

src/TMS_Kalman.cpp

Trusting you on this one

AsaNehm · 2024-07-11T13:41:18Z

src/TMS_Kalman.h

Trusting you on this one

AsaNehm

Looks good from my side

jdkio · 2024-07-24T19:19:13Z

I'm testing this branch to compare with main. I ran these three tests (with a limit of 5k files):

# Nersc files
ConvertToTMSTree.exe /pnfs/dune/persistent/users/abooth/Production/MiniProdN1p2-v1r1/run-spill-build/output/MiniProdN1p2_NDLAr_1E19_RHC.spill/EDEPSIM_SPILLS/00000/MiniProdN1p2_NDLAr_1E19_RHC.spill.00049.EDEPSIM_SPILLS.root
# events on TMS and LAr
ConvertToTMSTree.exe /pnfs/dune/persistent/users/kleykamp/nd_production/2024-04-18_rhc_test/edep/RHC/00m/00/antineutrino.0_1713482441.edep.root
# Lar-only but not from nersc
ConvertToTMSTree.exe /pnfs/dune/persistent/users/kleykamp/nd_production/2024-05-13_lar_only_rhc/edep/RHC/00m/00/antineutrino.0_1715654453.edep.root

They all process in main (tagged kleykamp_2024-07-24_main_test). I tried the same with this branch (tagged kleykamp_2024-07-24_3D-Kalman-fitter_test), and it gets stuck the second one which is TMS/LAr events: /pnfs/dune/persistent/users/kleykamp/nd_production/2024-04-18_rhc_test/edep/RHC/00m/00/antineutrino.0_1713482441.edep.root
The output stops at Processing slice 1 of event number 861 / 1842. I tried gdb's backtrace to see where it gets stuck, and it's consistently inside various parts of TrackMatching3D. Eg:

0x00007ffff7b1e3b4 in TMS_TrackFinder::TrackMatching3D (this=this@entry=0x16367c0 <TMS_TrackFinder::GetFinder()::Instance>)
    at TMS_Reco.cpp:1054
1054	                  || UTracks[itU].GetZ() < 11000 || UTracks[itU].GetZ() > 20000) --itU;
(gdb) bt
#0  0x00007ffff7b1e3b4 in TMS_TrackFinder::TrackMatching3D (this=this@entry=0x16367c0 <TMS_TrackFinder::GetFinder()::Instance>)
    at TMS_Reco.cpp:1054
#1  0x00007ffff7b288d6 in TMS_TrackFinder::FindTracks (this=this@entry=0x16367c0 <TMS_TrackFinder::GetFinder()::Instance>, event=...)
    at TMS_Reco.cpp:594
#2  0x0000000000408c4d in ConvertToTMSTree (filename=..., output_filename=...) at ../src/TMS_Reco.h:192
#3  0x0000000000407bd6 in main (argc=<optimized out>, argv=<optimized out>) at ConvertToTMSTree.cpp:251

I tried changing Run = false for the kalman filter and it gets stuck in the same location. I do see some changes in that code compared to main that are unaffected by the kalman filter, so maybe it's an issue with that logic. I'd be happy to run the test again if that helps

LiamOS · 2024-07-25T13:00:28Z

@AsaNehm good chance the above issue is related to the changes you made recently

AsaNehm · 2024-07-25T13:24:44Z

It might very well be.
@jdkio could you try whether the problem is still there if you add an if condition for all --itU and --itV for it to be above 0 (if (itV or itU > 0) --itV or --itU;) in my changed part of TMS_Reco.cpp [commits fixing matching with two hits per plane and potentially the one afterwards as well]. I had a problem with this yesterday as well and this seems to fix it for me

jdkio · 2024-07-25T16:32:43Z

It might very well be. @jdkio could you try whether the problem is still there if you add an if condition for all --itU and --itV for it to be above 0 (if (itV or itU > 0) --itV or --itU;) in my changed part of TMS_Reco.cpp [commits fixing matching with two hits per plane and potentially the one afterwards as well]. I had a problem with this yesterday as well and this seems to fix it for me

I'm not sure what you mean. Would you please submit the change?

AsaNehm · 2024-07-25T16:38:21Z

Did that. Please try it out

jdkio · 2024-07-31T16:09:28Z

This works now. I was able to test all 3 files and there was no infinite loop. Test files in
/exp/dune/data/users/kleykamp/dune-tms/simple_processing_test/kleykamp_2024-07-31_3D-Kalman-fitter_test

jdkio

Overall looks good, just some minor comments about verbosity and branch names

jdkio · 2024-07-31T16:12:33Z

src/TMS_Geom.h

+      TVector3 vec = TVector3(x,y,z);
+      //if (!IsInsideNearDetectorVolume(vec))
+        //std::cout << "Not in a detector?? " << x << ", " << y << ", " << z << std::endl;
+        //return NULL;
+


This code is doing nothing

jdkio · 2024-07-31T16:13:08Z

src/TMS_Geom.h

+//      std::cout << "[TMS_Geom.h] Getting materials between the points:" << std::endl
+//                  << "    point1: " << point1.X() << ", "<< point1.Y() << ", "<< point1.Z() << std::endl
+//                  << "    point2: " << point2.X() << ", "<< point2.Y() << ", "<< point2.Z() << std::endl;
+


Can remove this

jdkio · 2024-07-31T16:19:11Z

src/TMS_Reco.cpp

-    size_t nHits = xz_hits.size();
-    if (nHits < 1) continue; 
-    KalmanFitter = TMS_Kalman(xz_hits);
+      std::cout << "Kalman filter start pos : " << KalmanFilter.Start[0] << ", " << KalmanFilter.Start[1] << ", "  << KalmanFilter.Start[2] << std::endl;


Do we want it this verbose?

jdkio · 2024-07-31T16:21:15Z

src/TMS_TreeWriter.cpp

+  Reco_Tree->Branch("KalmanPos",     RecoTrackKalmanPos,     "TrackHitPos[nTracks][200][3]/F");
+  Reco_Tree->Branch("KalmanTruePos", RecoTrackKalmanTruePos, "TrackHitTruePos[nTracks][200][3]/F");
+  Reco_Tree->Branch("StartPos",      RecoTrackStartPos,      "StartPos[nTracks][3]/F");
+  Reco_Tree->Branch("StartDirection",RecoTrackStartDirection,"StartDirection[nTracks][3]/F");


In the future, we need to be careful about changing branch names. Downstream code uses Direction for StartDirection so it'll break scripts

jdkio · 2024-08-01T19:44:08Z

src/TMS_TreeWriter.cpp

+  Reco_Tree->Branch("nHits",         nHitsIn3DTrack,         "nHits[nTracks]/I");
+  Reco_Tree->Branch("TrackHitPos",   RecoTrackHitPos,        "TrackHitPos[nTracks][200][3]/F");
+  Reco_Tree->Branch("nKalmanNodes",  nKalmanNodes,           "nKalmanNodes[nTracks]/I");
+  Reco_Tree->Branch("KalmanPos",     RecoTrackKalmanPos,     "TrackHitPos[nTracks][200][3]/F");


I noticed TrackHitPos is repeated. Should it be
Reco_Tree->Branch("KalmanPos", RecoTrackKalmanPos, "KalmanPos[nTracks][200][3]/F");

jdkio · 2024-08-01T20:49:19Z

Looks like tracks start with unusual hit positions. Here are all hits on a track as an example. The first hit should be close to the second hit but the first hit has x outside the detector, and y = 0.

Looping over hits for track 0
(x, y, z)=(18314, 0, 15993)
(x, y, z)=(-1349.11, -1693.37, 15913)
(x, y, z)=(-1331.81, -1699.28, 15833)
(x, y, z)=(-1266.57, -1705.3, 15753)
(x, y, z)=(-1260.28, -1711.25, 15673

Looping over hits for track 0
(x, y, z)=(18314, 0, 18153)
(x, y, z)=(601.075, -1326.34, 18073)
(x, y, z)=(619.419, -1302.9, 17993)
(x, y, z)=(626.825, -1279.26, 17913)

The code is setting x is z_tms_end and y defaults to zero for the first hit of each track

jdkio · 2024-08-01T21:07:18Z

I also noticed the ends of tracks can be strange sometimes too

(x, y, z)=(1367.96, -61.3745, 13673)
(x, y, z)=(-1474.08, -778.715, 11643)
(x, y, z)=(-1474.08, -754.316, 11588)

jdkio · 2024-08-28T05:08:02Z

I made this comparison between PR 122 and PR 134: comparison.pdf. It's looking at plots from Tracking_Validation.cpp for this PR and what is currently main basically. Both have their pros and cons that need to be worked out. Direction became StartDirection and so the Direction and matching angle plots are bugged

I'm still concerned about hits that aren't on the track. I think that explains why we get this behavior:

But I do like the y comparison:

…n-clusters 125 potential problem with tracks in clusters

…direction

AsaNehm · 2024-09-10T22:06:49Z

Looking through the code on main there shouldn't be a problem with the shift to a PlaneByZ with coordinates instead of the plane number

LiamOS · 2024-10-22T16:10:03Z

Closing as this branch has become stale and isn't worth recovering. Not sure what happened and when, couldn't figure it out.

LiamOS requested review from jdkio and AsaNehm July 11, 2024 13:18

LiamOS self-assigned this Jul 11, 2024

LiamOS mentioned this pull request Jul 11, 2024

Finalise Kalman filter #1

Open

7 tasks

AsaNehm reviewed Jul 11, 2024

View reviewed changes

src/TMS_Kalman.cpp Outdated

Copy link

Contributor

AsaNehm Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trusting you on this one

AsaNehm reviewed Jul 11, 2024

View reviewed changes

src/TMS_Kalman.h Outdated

Copy link

Contributor

AsaNehm Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trusting you on this one

AsaNehm approved these changes Jul 11, 2024

View reviewed changes

jdkio approved these changes Jul 31, 2024

View reviewed changes

jdkio reviewed Aug 1, 2024

View reviewed changes

AsaNehm and others added 3 commits September 9, 2024 11:19

Merge pull request #148 from DUNE/125-potential-problem-with-tracks-i…

287980b

…n-clusters 125 potential problem with tracks in clusters

Take out the old direction which is superceeded by the Start- and End…

215d52a

…direction

Caught a duplication of code created by git in an attempt to merge

f947e78

jdkio mentioned this pull request Sep 18, 2024

Kalman filter in main already? #157

Closed

LiamOS added 2 commits September 18, 2024 09:01

Merge branch 'main' into feature/3D-Kalman-fitter

39f8114

Return in RunKalman to prevent crash from propagating outside detector

a813637

LiamOS closed this Oct 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/3D Kalman Filter #122

Feature/3D Kalman Filter #122

LiamOS commented Jul 11, 2024

AsaNehm Jul 11, 2024

AsaNehm Jul 11, 2024

AsaNehm left a comment

jdkio commented Jul 24, 2024

LiamOS commented Jul 25, 2024

AsaNehm commented Jul 25, 2024

jdkio commented Jul 25, 2024

AsaNehm commented Jul 25, 2024

jdkio commented Jul 31, 2024

jdkio left a comment

jdkio Jul 31, 2024

jdkio Jul 31, 2024

jdkio Jul 31, 2024

jdkio Jul 31, 2024

jdkio Aug 1, 2024

jdkio commented Aug 1, 2024

jdkio commented Aug 1, 2024

jdkio commented Aug 28, 2024

AsaNehm commented Sep 10, 2024

LiamOS commented Oct 22, 2024

Feature/3D Kalman Filter #122

Feature/3D Kalman Filter #122

Conversation

LiamOS commented Jul 11, 2024

AsaNehm Jul 11, 2024

Choose a reason for hiding this comment

AsaNehm Jul 11, 2024

Choose a reason for hiding this comment

AsaNehm left a comment

Choose a reason for hiding this comment

jdkio commented Jul 24, 2024

LiamOS commented Jul 25, 2024

AsaNehm commented Jul 25, 2024

jdkio commented Jul 25, 2024

AsaNehm commented Jul 25, 2024

jdkio commented Jul 31, 2024

jdkio left a comment

Choose a reason for hiding this comment

jdkio Jul 31, 2024

Choose a reason for hiding this comment

jdkio Jul 31, 2024

Choose a reason for hiding this comment

jdkio Jul 31, 2024

Choose a reason for hiding this comment

jdkio Jul 31, 2024

Choose a reason for hiding this comment

jdkio Aug 1, 2024

Choose a reason for hiding this comment

jdkio commented Aug 1, 2024

jdkio commented Aug 1, 2024

jdkio commented Aug 28, 2024

AsaNehm commented Sep 10, 2024

LiamOS commented Oct 22, 2024