From e0c779c992c5dbd09a8d08af980799f09068976b Mon Sep 17 00:00:00 2001 From: Sebastien Ponce <sebastien.ponce@cern.ch> Date: Tue, 5 Sep 2023 15:14:54 +0200 Subject: [PATCH] Fixed float comparisons in histograms --- GaudiCommonSvc/src/HistogramSvc/H1D.cpp | 24 ++++++----------- GaudiCommonSvc/src/HistogramSvc/H2D.cpp | 29 ++++++++------------- GaudiCommonSvc/src/HistogramSvc/H3D.cpp | 34 ++++++++++--------------- GaudiCommonSvc/src/HistogramSvc/P1D.cpp | 2 +- 4 files changed, 33 insertions(+), 56 deletions(-) diff --git a/GaudiCommonSvc/src/HistogramSvc/H1D.cpp b/GaudiCommonSvc/src/HistogramSvc/H1D.cpp index e8b4be233b..a11d4e0bbd 100644 --- a/GaudiCommonSvc/src/HistogramSvc/H1D.cpp +++ b/GaudiCommonSvc/src/HistogramSvc/H1D.cpp @@ -125,22 +125,17 @@ bool Gaudi::Histogram1D::setBinContents( int i, int entries, double height, doub return true; } -#ifdef __ICC -// disable icc remark #1572: floating-point equality and inequality comparisons are unreliable -// The comparison are meant -# pragma warning( push ) -# pragma warning( disable : 1572 ) -#endif bool Gaudi::Histogram1D::setRms( double rms ) { m_rep->SetEntries( m_sumEntries ); std::vector<double> stat( 11 ); // sum weights stat[0] = sumBinHeights(); stat[1] = 0; - if ( equivalentBinEntries() != 0 ) stat[1] = ( sumBinHeights() * sumBinHeights() ) / equivalentBinEntries(); + if ( abs( equivalentBinEntries() ) > std::numeric_limits<double>::epsilon() ) + stat[1] = ( sumBinHeights() * sumBinHeights() ) / equivalentBinEntries(); stat[2] = m_sumwx; double mean = 0; - if ( sumBinHeights() != 0 ) mean = m_sumwx / sumBinHeights(); + if ( abs( sumBinHeights() ) > std::numeric_limits<double>::epsilon() ) mean = m_sumwx / sumBinHeights(); stat[3] = ( mean * mean + rms * rms ) * sumBinHeights(); m_rep->PutStats( &stat.front() ); return true; @@ -155,7 +150,8 @@ bool Gaudi::Histogram1D::setStatistics( int allEntries, double eqBinEntries, dou stat[0] = sumBinHeights(); // sum weights **2 stat[1] = 0; - if ( eqBinEntries != 0 ) stat[1] = ( sumBinHeights() * sumBinHeights() ) / eqBinEntries; + if ( abs( eqBinEntries ) > std::numeric_limits<double>::epsilon() ) + stat[1] = ( sumBinHeights() * sumBinHeights() ) / eqBinEntries; // sum weights * x stat[2] = mean * sumBinHeights(); // sum weight * x **2 @@ -167,7 +163,7 @@ bool Gaudi::Histogram1D::setStatistics( int allEntries, double eqBinEntries, dou bool Gaudi::Histogram1D::fill( double x, double weight ) { // avoid race conditions when filling the histogram auto guard = std::scoped_lock{ m_fillSerialization }; - ( weight == 1. ) ? m_rep->Fill( x ) : m_rep->Fill( x, weight ); + m_rep->Fill( x, weight ); return true; } @@ -192,7 +188,8 @@ void Gaudi::Histogram1D::copyFromAida( const AIDA::IHistogram1D& h ) { double sumw = h.sumBinHeights(); // sumw2 double sumw2 = 0; - if ( h.equivalentBinEntries() != 0 ) sumw2 = ( sumw * sumw ) / h.equivalentBinEntries(); + if ( abs( h.equivalentBinEntries() ) > std::numeric_limits<double>::epsilon() ) + sumw2 = ( sumw * sumw ) / h.equivalentBinEntries(); double sumwx = h.mean() * h.sumBinHeights(); double sumwx2 = ( h.mean() * h.mean() + h.rms() * h.rms() ) * h.sumBinHeights(); @@ -215,11 +212,6 @@ void Gaudi::Histogram1D::copyFromAida( const AIDA::IHistogram1D& h ) { m_rep->PutStats( &stat.front() ); } -#ifdef __ICC -// re-enable icc remark #1572 -# pragma warning( pop ) -#endif - StreamBuffer& Gaudi::Histogram1D::serialize( StreamBuffer& s ) { // DataObject::serialize(s); std::string title; diff --git a/GaudiCommonSvc/src/HistogramSvc/H2D.cpp b/GaudiCommonSvc/src/HistogramSvc/H2D.cpp index 5841f9d383..3033d1d99a 100644 --- a/GaudiCommonSvc/src/HistogramSvc/H2D.cpp +++ b/GaudiCommonSvc/src/HistogramSvc/H2D.cpp @@ -173,16 +173,10 @@ bool Gaudi::Histogram2D::reset() { return Base::reset(); } -#ifdef __ICC -// disable icc remark #1572: floating-point equality and inequality comparisons are unreliable -// The comparison are meant -# pragma warning( push ) -# pragma warning( disable : 1572 ) -#endif bool Gaudi::Histogram2D::fill( double x, double y, double weight ) { // avoid race conditiosn when filling the histogram auto guard = std::scoped_lock{ m_fillSerialization }; - ( weight == 1. ) ? m_rep->Fill( x, y ) : m_rep->Fill( x, y, weight ); + m_rep->Fill( x, y, weight ); return true; } @@ -191,14 +185,17 @@ bool Gaudi::Histogram2D::setRms( double rmsX, double rmsY ) { std::vector<double> stat( 11 ); stat[0] = sumBinHeights(); stat[1] = 0; - if ( equivalentBinEntries() != 0 ) stat[1] = ( sumBinHeights() * sumBinHeights() ) / equivalentBinEntries(); + if ( abs( equivalentBinEntries() ) > std::numeric_limits<double>::epsilon() ) + stat[1] = ( sumBinHeights() * sumBinHeights() ) / equivalentBinEntries(); stat[2] = m_sumwx; - double meanX = 0; - if ( sumBinHeights() != 0 ) meanX = m_sumwx / sumBinHeights(); - stat[3] = ( meanX * meanX + rmsX * rmsX ) * sumBinHeights(); stat[4] = m_sumwy; + double meanX = 0; double meanY = 0; - if ( sumBinHeights() != 0 ) meanY = m_sumwy / sumBinHeights(); + if ( abs( sumBinHeights() ) > std::numeric_limits<double>::epsilon() ) { + meanX = m_sumwx / sumBinHeights(); + meanY = m_sumwy / sumBinHeights(); + } + stat[3] = ( meanX * meanX + rmsX * rmsX ) * sumBinHeights(); stat[5] = ( meanY * meanY + rmsY * rmsY ) * sumBinHeights(); stat[6] = 0; m_rep->PutStats( &stat.front() ); @@ -231,7 +228,8 @@ void Gaudi::Histogram2D::copyFromAida( const IHistogram2D& h ) { // statistics double sumw = h.sumBinHeights(); double sumw2 = 0; - if ( h.equivalentBinEntries() != 0 ) sumw2 = ( sumw * sumw ) / h.equivalentBinEntries(); + if ( abs( h.equivalentBinEntries() ) > std::numeric_limits<double>::epsilon() ) + sumw2 = ( sumw * sumw ) / h.equivalentBinEntries(); double sumwx = h.meanX() * h.sumBinHeights(); double sumwx2 = ( h.meanX() * h.meanX() + h.rmsX() * h.rmsX() ) * h.sumBinHeights(); double sumwy = h.meanY() * h.sumBinHeights(); @@ -255,8 +253,3 @@ void Gaudi::Histogram2D::copyFromAida( const IHistogram2D& h ) { std::array<double, 11> stat = { { sumw, sumw2, sumwx, sumwx2, sumwy, sumwy2, sumwxy } }; m_rep->PutStats( stat.data() ); } - -#ifdef __ICC -// re-enable icc remark #1572 -# pragma warning( pop ) -#endif diff --git a/GaudiCommonSvc/src/HistogramSvc/H3D.cpp b/GaudiCommonSvc/src/HistogramSvc/H3D.cpp index 67a3509878..c6f7f79492 100644 --- a/GaudiCommonSvc/src/HistogramSvc/H3D.cpp +++ b/GaudiCommonSvc/src/HistogramSvc/H3D.cpp @@ -131,34 +131,30 @@ void* Gaudi::Histogram3D::cast( const std::string& className ) const { return nullptr; } -#ifdef __ICC -// disable icc remark #1572: floating-point equality and inequality comparisons are unreliable -// The comparison are meant -# pragma warning( push ) -# pragma warning( disable : 1572 ) -#endif bool Gaudi::Histogram3D::setRms( double rmsX, double rmsY, double rmsZ ) { m_rep->SetEntries( m_sumEntries ); std::vector<double> stat( 11 ); // sum weights stat[0] = sumBinHeights(); stat[1] = 0; - if ( equivalentBinEntries() != 0 ) stat[1] = ( sumBinHeights() * sumBinHeights() ) / equivalentBinEntries(); + if ( abs( equivalentBinEntries() ) > std::numeric_limits<double>::epsilon() ) + stat[1] = ( sumBinHeights() * sumBinHeights() ) / equivalentBinEntries(); stat[2] = m_sumwx; - double meanX = 0; - if ( sumBinHeights() != 0 ) meanX = m_sumwx / sumBinHeights(); - stat[3] = ( meanX * meanX + rmsX * rmsX ) * sumBinHeights(); stat[4] = m_sumwy; - double meanY = 0; - if ( sumBinHeights() != 0 ) meanY = m_sumwy / sumBinHeights(); - stat[5] = ( meanY * meanY + rmsY * rmsY ) * sumBinHeights(); stat[6] = 0; stat[7] = m_sumwz; + double meanX = 0; + double meanY = 0; double meanZ = 0; - if ( sumBinHeights() != 0 ) meanZ = m_sumwz / sumBinHeights(); + if ( abs( sumBinHeights() ) > std::numeric_limits<double>::epsilon() ) { + meanX = m_sumwx / sumBinHeights(); + meanY = m_sumwy / sumBinHeights(); + meanZ = m_sumwz / sumBinHeights(); + } + stat[3] = ( meanX * meanX + rmsX * rmsX ) * sumBinHeights(); + stat[5] = ( meanY * meanY + rmsY * rmsY ) * sumBinHeights(); stat[8] = ( meanZ * meanZ + rmsZ * rmsZ ) * sumBinHeights(); // do not need to use sumwxy sumwxz and sumwyz - m_rep->PutStats( &stat.front() ); return true; } @@ -198,7 +194,8 @@ void Gaudi::Histogram3D::copyFromAida( const AIDA::IHistogram3D& h ) { // statistics double sumw = h.sumBinHeights(); double sumw2 = 0; - if ( h.equivalentBinEntries() != 0 ) sumw2 = ( sumw * sumw ) / h.equivalentBinEntries(); + if ( abs( h.equivalentBinEntries() ) > std::numeric_limits<double>::epsilon() ) + sumw2 = ( sumw * sumw ) / h.equivalentBinEntries(); double sumwx = h.meanX() * h.sumBinHeights(); double sumwx2 = ( h.meanX() * h.meanX() + h.rmsX() * h.rmsX() ) * h.sumBinHeights(); double sumwy = h.meanY() * h.sumBinHeights(); @@ -243,8 +240,3 @@ void Gaudi::Histogram3D::copyFromAida( const AIDA::IHistogram3D& h ) { stat[10] = sumwyz; m_rep->PutStats( &stat.front() ); } - -#ifdef __ICC -// re-enable icc remark #1572 -# pragma warning( pop ) -#endif diff --git a/GaudiCommonSvc/src/HistogramSvc/P1D.cpp b/GaudiCommonSvc/src/HistogramSvc/P1D.cpp index 603369da0e..3a6769bddb 100644 --- a/GaudiCommonSvc/src/HistogramSvc/P1D.cpp +++ b/GaudiCommonSvc/src/HistogramSvc/P1D.cpp @@ -119,6 +119,6 @@ bool Gaudi::Profile1D::setBinContents( int i, int entries, double height, double bool Gaudi::Profile1D::fill( double x, double y, double weight ) { // avoid race conditions when filling the profile auto guard = std::scoped_lock{ m_fillSerialization }; - ( weight == 1. ) ? m_rep->Fill( x, y ) : m_rep->Fill( x, y, weight ); + m_rep->Fill( x, y, weight ); return true; } -- GitLab