Avoid Nans in primary vertex #5490
Conversation
|
A new Pull Request was created by @VinInn (Vincenzo Innocente) for CMSSW_7_3_X. Avoid Nans in primary vertex It involves the following packages: RecoVertex/AdaptiveVertexFit @cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks. |
| }; | ||
|
|
||
| if ( (**i).weight() >= theWeightThreshold ) ns_trks++; | ||
|
|
||
| if ( fabs ( nVertex.position().z() ) > 10000. || |
There was a problem hiding this comment.
probably it's a silly point,
but could we get rid of such hard-coded numbers ?
There was a problem hiding this comment.
If somebody volunteers (the code was there already)
| double t_z = ((*it).stateAtBeamLine().trackStateAtPCA()).position().z(); | ||
| double phi=((*it).stateAtBeamLine().trackStateAtPCA()).momentum().phi(); | ||
| double tantheta = std::tan( ((*it).stateAtBeamLine().trackStateAtPCA()).momentum().theta()); | ||
| if (std::abs(t_z) > 1000.) continue; |
There was a problem hiding this comment.
This version of "1000." in the middle of the code is new. I would write it as a const with a suggestive name.
|
@cmsbuild: tests, comparison? |
|
Pull request #5490 was updated. @cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please check and sign again. |
|
As expected the main changes are to the pixel vertices, less for PV. I see some changes to CSV b-tagging (which was "more sensitive" I was told and not the main algo). I tested in wf202 (ttbar with PU) mainly for now. Below some results (red is this PR vs recent IB), please comment. Overall I see slight decrease in pixel vertices (and related objects), I thougt it had to be the opposite.
|
|
@VinInn @StoyanStoynev |
|
It is consistent with what I observe as well. |
|
+1 |
|
sorry, was wrong noise on my side. |
RecoVertex -- Avoid Nans in primary vertex
| void sortTracksByPt(std::vector<reco::TransientTrack> & cont) { | ||
| auto s = cont.size(); | ||
| float pt2[s]; int ind[s]; int i=0; | ||
| for (auto const & tk : cont) { ind[i]=i; pt2[++i] = tk.impactPointState().globalMomentum().perp2();} |
There was a problem hiding this comment.
this should have been i++, or, more straightforward and less error prone, pt2[i] = ...; ++i;
Valgrind reports errors here.
I suspect this is where our vertex/btag discrepancies originate.
There was a problem hiding this comment.
I think this is only in 73X
While investigating the irreproducibility of relvals I discovered that in some cases no PixelVertices were reconstructed. I trace this down to some Nans.
Here I try to trap Nans and conditions that will generate Nans.
The effect on OfflineVertices is essentially negligible while evident to PixelVertices.
This is also one of the reason that prompted #5476
Few more optimizations have been added to the algorithms.
AdaptiveVertexFitter and KalmanVertexFitter (and the infrastructure used in there) would benefit of a complete re-engineering in light of C++11 and vectorization.
@mtosi, could you please test this for HLT?