Fix legacy vertexing: drop ill-conditioned tracks and skip unneeded momenta-at-vertex computation - #525
Open
kjvbrt wants to merge 5 commits into
Open
Conversation
Delphes' VertexFit silently returns a NaN vertex (position and chi2) when fed a track whose covariance matrix is not positive-definite, instead of failing loudly, while also flooding stdout trying (and failing) to invert it. In get_PrimaryTracks() this additionally broke the chi2-based outlier removal loop, since comparisons against a NaN chi2 are always false, so degenerate tracks were silently never pruned. Filter such tracks out before they reach VertexFit, using Delphes' own TrkUtil::CheckPosDef() guard, and log which track parameters were dropped and why via stderr (unaffected by the stdout suppression already wrapping the fit). Also extends the existing stdout suppression (introduced in HEP-FCC#379) to get_PrimaryTracks(), which was never covered by it -- this is likely a contributor to the severe slowdowns reported after that fix landed, since get_PrimaryTracks() is what the vertexing tutorial and typical analyses actually call. Fixes HEP-FCC#378
kjvbrt
changed the title
Profiling on a real winter2023 Zbb_EvtGen_Bs2Inclusive sample showed that the covariance-matrix fix alone barely moved the needle on the reported slowdown: constructing Delphes' VertexMore (to fill updated_track_momentum_at_vertex) dominates VertexFitter_Tk's runtime, scaling roughly as O(ntracks^3), while the fit itself and the plain per-track parameter readout are comparatively cheap. Measured on 300 events single-threaded, one call alone cost 429ms-1216ms depending on track multiplicity, versus a few tens of ms for the fit. Add a ComputeMomentaAtVertex flag (default true, preserving current behavior) so callers that only need the vertex position/chi2 -- like the primary-vertex tutorial, which never reads the per-track momenta it discards -- can skip that cost. On the same 300-event sample this takes VertexFitter_Tk's total contribution from ~161s down to ~58s.
kjvbrt
changed the title
…_VertexObject() The reported b2snunu slowdown goes through myUtils::get_VertexObject() (called once per MC vertex per event) -> VertexFitterSimple::VertexFitter(), not VertexFitter_Tk() directly, so the previous commit's flag was unreachable from that code path. hasPV(), the actual consumer in the reported reproducer, only ever reads vertex.primary and never the per-track momenta. Verified on a real winter2023 Bs2Inclusive file that hasPV() output is bit-identical with ComputeMomentaAtVertex=false (0 mismatches across 299 surviving events out of 300), while wall time for that reproducer drops from 22s to 10s single-threaded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #378. Two issues found while digging into the reported vertexing errors/slowdown:
VertexFit(Delphes) silently returns a NaN vertex when fed a track with a non-positive-definite covariance matrix, and this brokeget_PrimaryTracks()'s chi2-based outlier pruning (NaN comparisons are always false). Now such tracks are dropped before the fit, with the reason logged viastderr. Also extends thestdoutsuppression from Suppressing printf output from TMatrixBase #379 toget_PrimaryTracks(), which it never covered.VertexFitter_Tkunconditionally builds a DelphesVertexMoreto compute per-track momenta at the vertex, which scales ~O(ntracks^3) and dominates runtime for events with many tracks -- even when nothing reads that output. Added aComputeMomentaAtVertexflag (defaulttrue, no behavior change for existing callers) and set it tofalsein the vertexing tutorial and inmyUtils::get_VertexObject()/VertexFitter(), which is what the originally-reported b2snunu slowdown actually goes through.Verification
On a real winter2023
Bs2Inclusivefile, single-threaded: the tutorial's 300 events went from 161s to 58s, and the original issue's minimal repro (get_VertexObject->hasPV->Filter) went from 22s to 10s with bit-identicalEVT_hasPVoutput. The NaN-vertex fix was verified with synthetic degenerate-covariance tracks (see commit messages for exact before/after numbers).