search

trackreco / mkFit

Vectorized, Parallelized Tracking
https://trackreco.github.io/
Apache License 2.0
17 stars 15 forks source link

Add filter to remove nan and silly candidates #374

Closed mmasciov closed 2 years ago

mmasciov commented 2 years ago

PR description

This PR introduces a filter on 'silly' candidates, to avoid error/warning messages in CMSSW, addressing #373.

PR validation

Physics performance is unchanged. Timing performance is also ~ unchanged (in the plots linked below there is an offset in the measurement; however, after subtracting such offset, timing is indeed unchanged). MTV results based on TTbar (=50): http://uaf-10.t2.ucsd.edu/~mmasciov/MkFit_devs/MTV_testNanNSillyRemoval/ Running on 100 events, the size of the log file decreases from 3.6 MB to 1.3 MB (note that the warning suppression introduced by Slava in cms-sw/cmssw#35802 is not included). The instances of 'candidate ignored' decrease from 4778 to 53.

osschar commented 2 years ago

Filter-style removal makes sense between forward-search and backward fit. It's somewhat expensive as it has to move comb-cands up in the vector once one of them gets removed. We do run all filters at the same time, don't we?

I thought we need to to do this at the very end ... so why not just have the check in export function ... and simply not export the CombCand for which this fails.

How about seeds? Do we want to check there, too? There was a "fix" option (that didn't make much sense to me and I remember asking what should actually be done with those seeds / or what beter values to put there would be / or where this would need to be fixed upstream (in the seed producer)).

mmasciov commented 2 years ago

Filter-style removal makes sense between forward-search and backward fit. It's somewhat expensive as it has to move comb-cands up in the vector once one of them gets removed. We do run all filters at the same time, don't we?

I thought we need to to do this at the very end ... so why not just have the check in export function ... and simply not export the CombCand for which this fails.

How about seeds? Do we want to check there, too? There was a "fix" option (that didn't make much sense to me and I remember asking what should actually be done with those seeds / or what beter values to put there would be / or where this would need to be fixed upstream (in the seed producer)).

This is done at the very end, after backward fit: https://github.com/trackreco/mkFit/pull/374/files#diff-9bd117b4474e1261389e0f33d9f898dc985c780ab0e7c9c12339ce68867a0e66R715 There are also new filters applied at the end. It could probably be done also during the export, I guess.

As for seeds, currently we assign an error of 1e-5. We could try to remove the bad seeds instead (there is an option, disabled by default, which can remove silly seeds in the seed post-processing, instead if 'fixing' them.

osschar commented 2 years ago

OK, then let's leave the final filter optimization for next time (maybe passing a vector of filter functions to export) :)

I don't know what to do with bad seeds ... and how many of them there even are in each iteration.

mmasciov commented 2 years ago

OK, then let's leave the final filter optimization for next time (maybe passing a vector of filter functions to export) :)

I don't know what to do with bad seeds ... and how many of them there even are in each iteration.

I have enabled by default the removal of silly seeds with commit 9ef01f6, if any. No difference, as this is very unlikely to happen: http://uaf-10.t2.ucsd.edu/~mmasciov/MkFit_devs/MTV_testNanNSillyRemoval_sillySeedRemoval/