From ea5055219f9bd6eab68cc2d5b3bef3b502afe26a Mon Sep 17 00:00:00 2001 From: Louis Henry <louis.henry@cern.ch> Date: Thu, 14 Nov 2019 17:00:24 +0100 Subject: [PATCH 1/7] Corrected a rare instance of segmentation fault due to chi2 being rigourously 0 --- Pr/PrAlgorithms/src/PrHybridSeeding.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp index f1344f4fe38..5c674f0c2d5 100644 --- a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp +++ b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp @@ -802,15 +802,15 @@ bool PrHybridSeeding::createTracksFromHough( const unsigned int& //---LoH: could probably be done in one pass with minimal loss while ( itEnd2 - itBeg2 > nLay ) { auto worstMultiple = itEnd2; - float worstchi = std::numeric_limits<float>::min(); + float worstchi = 0.f; float chi_hit; for ( auto line_hit = itBeg2; line_hit != itEnd2; ++line_hit ) { if ( plCount.nbInPlane( line_hit->planeCode ) == 1 ) continue; chi_hit = BestLine.chi2hit( *line_hit ); - if ( chi_hit > worstchi ) { + if ( chi_hit >= worstchi ) { worstchi = chi_hit; worstMultiple = line_hit; - } + } } // it will just change the m_planeCode[int] not the whole internal hit counting ! ) plCount.removeHitInPlane( worstMultiple->planeCode ); -- GitLab From aa8be4b75e174afae0433a9d2b3e1621f3580d42 Mon Sep 17 00:00:00 2001 From: Gitlab CI <noreply@cern.ch> Date: Thu, 14 Nov 2019 16:01:02 +0000 Subject: [PATCH 2/7] Fixed formatting patch generated by https://gitlab.cern.ch/lhcb/Rec/-/jobs/6165265 --- Pr/PrAlgorithms/src/PrHybridSeeding.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp index 5c674f0c2d5..a0270d55d47 100644 --- a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp +++ b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp @@ -802,15 +802,15 @@ bool PrHybridSeeding::createTracksFromHough( const unsigned int& //---LoH: could probably be done in one pass with minimal loss while ( itEnd2 - itBeg2 > nLay ) { auto worstMultiple = itEnd2; - float worstchi = 0.f; + float worstchi = 0.f; float chi_hit; for ( auto line_hit = itBeg2; line_hit != itEnd2; ++line_hit ) { if ( plCount.nbInPlane( line_hit->planeCode ) == 1 ) continue; chi_hit = BestLine.chi2hit( *line_hit ); - if ( chi_hit >= worstchi ) { + if ( chi_hit >= worstchi ) { worstchi = chi_hit; worstMultiple = line_hit; - } + } } // it will just change the m_planeCode[int] not the whole internal hit counting ! ) plCount.removeHitInPlane( worstMultiple->planeCode ); -- GitLab From a4213a8f9f75b1448f9545472a33ed649396a96b Mon Sep 17 00:00:00 2001 From: Louis Henry <louis.henry@cern.ch> Date: Thu, 14 Nov 2019 19:51:51 +0000 Subject: [PATCH 3/7] Update PrHybridSeeding.cpp --- Pr/PrAlgorithms/src/PrHybridSeeding.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp index a0270d55d47..1e3f079676c 100644 --- a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp +++ b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp @@ -802,12 +802,12 @@ bool PrHybridSeeding::createTracksFromHough( const unsigned int& //---LoH: could probably be done in one pass with minimal loss while ( itEnd2 - itBeg2 > nLay ) { auto worstMultiple = itEnd2; - float worstchi = 0.f; + float worstchi = std::numeric_limits<float>::lowest(); float chi_hit; for ( auto line_hit = itBeg2; line_hit != itEnd2; ++line_hit ) { if ( plCount.nbInPlane( line_hit->planeCode ) == 1 ) continue; chi_hit = BestLine.chi2hit( *line_hit ); - if ( chi_hit >= worstchi ) { + if ( chi_hit > worstchi ) { worstchi = chi_hit; worstMultiple = line_hit; } -- GitLab From 21c1d777e5b9d9909c9a8daf68861e7058b53b66 Mon Sep 17 00:00:00 2001 From: Louis Henry <louis.henry@cern.ch> Date: Mon, 18 Nov 2019 13:13:08 +0000 Subject: [PATCH 4/7] Apply suggestion to Pr/PrAlgorithms/src/PrHybridSeeding.cpp --- Pr/PrAlgorithms/src/PrHybridSeeding.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp index 1e3f079676c..cecd61ba1e5 100644 --- a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp +++ b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp @@ -825,7 +825,7 @@ bool PrHybridSeeding::createTracksFromHough( const unsigned int& fit &= LineOK( minChi2LineLow, minChi2LineHigh, xCandidate, BestLine, nInit ); if ( !fit ) continue; trackCand.setFromXCand( xCandidate ); - for ( auto hit = itBeg2; hit < itEnd2; hit++ ) trackCand.hits().push_back( *hit ); + trackCand.hits.insert( trackCand.hits.end(), itBeg2, itEnd2 ); } trackCand.setYParam( BestLine.ay(), BestLine.by() ); //---LoH: performs the simultaneous fit -- GitLab From 1a8deb812ebfc1d7d0d26e0acfb06c3bb6ad12c3 Mon Sep 17 00:00:00 2001 From: Louis Henry <louis.henry@cern.ch> Date: Wed, 27 Nov 2019 09:01:55 +0000 Subject: [PATCH 5/7] Update PrHybridSeeding.cpp --- Pr/PrAlgorithms/src/PrHybridSeeding.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp index cecd61ba1e5..f8a1ae3cbf1 100644 --- a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp +++ b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp @@ -825,7 +825,7 @@ bool PrHybridSeeding::createTracksFromHough( const unsigned int& fit &= LineOK( minChi2LineLow, minChi2LineHigh, xCandidate, BestLine, nInit ); if ( !fit ) continue; trackCand.setFromXCand( xCandidate ); - trackCand.hits.insert( trackCand.hits.end(), itBeg2, itEnd2 ); + trackCand.hits().insert( trackCand.hits.end(), itBeg2, itEnd2 ); } trackCand.setYParam( BestLine.ay(), BestLine.by() ); //---LoH: performs the simultaneous fit -- GitLab From deb550dfad2eaf2c7475cdffdbf4c72ad5281fa4 Mon Sep 17 00:00:00 2001 From: Louis Henry <louis.henry@cern.ch> Date: Wed, 27 Nov 2019 10:09:38 +0100 Subject: [PATCH 6/7] Corrected error that prevented compilation --- Pr/PrAlgorithms/src/PrHybridSeeding.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp index f8a1ae3cbf1..c0a7719bf86 100644 --- a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp +++ b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp @@ -825,7 +825,7 @@ bool PrHybridSeeding::createTracksFromHough( const unsigned int& fit &= LineOK( minChi2LineLow, minChi2LineHigh, xCandidate, BestLine, nInit ); if ( !fit ) continue; trackCand.setFromXCand( xCandidate ); - trackCand.hits().insert( trackCand.hits.end(), itBeg2, itEnd2 ); + trackCand.hits().insert( trackCand.hits().end(), itBeg2, itEnd2 ); } trackCand.setYParam( BestLine.ay(), BestLine.by() ); //---LoH: performs the simultaneous fit -- GitLab From 09a3cefb632c167239e3caafaa66f5c1d01ff87a Mon Sep 17 00:00:00 2001 From: Rosen Matev <rosen.matev@cern.ch> Date: Fri, 29 Nov 2019 08:40:38 +0100 Subject: [PATCH 7/7] Check before dereferencing iterator --- Pr/PrAlgorithms/src/PrHybridSeeding.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp index c0a7719bf86..abf1c085407 100644 --- a/Pr/PrAlgorithms/src/PrHybridSeeding.cpp +++ b/Pr/PrAlgorithms/src/PrHybridSeeding.cpp @@ -812,6 +812,11 @@ bool PrHybridSeeding::createTracksFromHough( const unsigned int& worstMultiple = line_hit; } } + if ( worstMultiple == itEnd2 ) { + // This should probably never be reached, see + // https://gitlab.cern.ch/lhcb/Rec/merge_requests/1788#note_2987027 + break; + } // it will just change the m_planeCode[int] not the whole internal hit counting ! ) plCount.removeHitInPlane( worstMultiple->planeCode ); // Avoid relocations by swapping the worst hit to the last hit and reducing the end by one. -- GitLab