project8 / katydid

Project 8 data analysis package
Other
5 stars 4 forks source link

Time jump tolerance #83

Closed guiguem closed 7 years ago

guiguem commented 7 years ago

The event I am considering is long (see attached pdf) and composed of several tracks and the algorithm at some point reconstructs it as 2 events. It seems that, even the elapsed time between the two events is less than 120 microsec (smaller than the 0.00184 — I guess 1.84 ms— set in the config file), the event builder does not put them together. Maybe I missed something (like the time differences are not in seconds) or there is a bug, which might be worth investigating, since this can lead to misconstructions. If it is the first, switching back to standard time or number of bins might be helpful. ReconstructedEvent.pdf

laroque commented 7 years ago

I don't see an attachment. It would be helpful to know the exact data (egg or mat) file and katydid config file you're using so that we can quickly reproduce the behavior.

evzayas commented 7 years ago

@guigue What is the difference between the red and blue dots on the attached plot? And @laroque, I believe the MAT file used is rid000001097-2016.09.13.22.55.32.411, which I can't attach because github doesn't support it. Mathieu can confirm that's the right one. I will look into this today.

evzayas commented 7 years ago

I think I understand partially what's going on. In this MAT file there are 15 tracks total, but they are being combined into two events each with one single MPT (i.e. group of parallel tracks). The first has multiplicity 8 and the second has multiplicity 7. We can see from the plot Mathieu included that this is very wrong.

The reason for this is the sideband time tolerance, which is set in the config to 1.84ms. This is way too large; the original config file with Mathieu's optimized parameters had an extra zero and I never caught it. This should instead be set to 0.184ms as Mathieu says in his writeup of the reconstruction algorithm.

The event builder uses the average start/end times in a MPT to combine events, but with this high tolerance we got MPTs that spanned several milliseconds and the "average" start time kept drifting toward the middle of the track grouping. Once the difference between the middle of the group and the start time of the next track finally exceeded 1.84ms (notice the separation between the two big blue dots is almost exactly 1.84ms x 2), a new MPT was created and these two large MPTs just became the two events.

evzayas commented 7 years ago

and by had an extra zero, I meant it needs an extra zero.

With the parameters fixed, I instead get 14 events from the 15 tracks. Two are combined into one MPT, and none of the MPTs are combined into events.

@laroque, I'd like to call your attention to line 140 of the MPEB:

if ( trackIt->fMeanEndTimeInRunC - *endTimeIt < fJumpTimeTolerance )
                        { // if this track head matches the tail of a track in this event, add it

Why do you use fMeanEndTimeInRunC? Shouldn't it be the start time you're comparing to the end time of the current event? This behavior makes me think nothing will ever be added to an active event, which seems to be the case.

I have also confirmed that this line is identical in the old version of the MPEB.

evzayas commented 7 years ago

katydid-log-fixed.txt katydid-log-old.txt

For anyone who is curious here are the the relevant parts of the log output from Katydid with 1.84ms (old) and 0.184ms @(fixed).

evzayas commented 7 years ago

Sorry for the spam but I'll continue to update as I figure this out. I changed line 140 above to use fMeanStartTimeInRunC rather than the end time, and now the file produces 5 events. This result makes sense given the parameters we have specified; I am uploading a diagram to explain it as best I can. The two tracks between 4 and 5 milliseconds are combined into a MPT (which they should not be), and this causes a break in the event sequence. In addition, a couple of the tracks towards the end have a separation slightly greater than 0.184ms, so they are split up as well.

At this point the "problem" is really just a result of the event builder being pretty basic, and the parameters (perhaps) not perfect. But these are things we are well aware of. It seems to me that the event builder is functioning as we expect as this point; I would still like Ben to confirm that the change I made is the right one.

20170201_130514

guiguem commented 7 years ago

The blue are the reconstructed starting times and the red the ending times.

evzayas commented 7 years ago

After discussion with Ben, we have confirmed the bug in the event builder's head-tail comparison logic. A fix is now in the branch hotfix/EventBuilderLogic. I believe the behavior of the event builder (past and present) is now fully understood, and the output of the current version is consistent with expectations.

@guigue, I would suggest that you check out this new version and see if you agree with that assessment. Feel free to ping me if anything about it is still confusing.

evzayas commented 7 years ago

Closing the issue as the hotfix has been merged into master.

guigue commented 7 years ago

I started to receive emails from this project, that I don't belong. Please remove the cc: to guigue@craam.mackenzie.br (my professional address). Thank you very much.

Guillermo

nsoblath commented 7 years ago

Hi Guillermo. I apologize for all of the spam emails from a project that you don't belong to. We have someone on our project with a very similar username. We'll be more careful in the future to avoid this problem.

guigue commented 7 years ago

Thank you very much for your quick reaction Noah. And, please, don't worry. In fact, I was a little worry and wanted to warm you because the real guigue of the project could be missing messages.

Regards, Guillermo