From fe3d9c22e6cc37327c9bf4b90bb0d1ae302d50b5 Mon Sep 17 00:00:00 2001
From: Sebastien Ponce <sebastien.ponce@cern.ch>
Date: Tue, 5 Sep 2023 16:23:39 +0200
Subject: [PATCH] Fixed float comparisons in HistoDump.cpp

---
 GaudiAlg/tests/qmtest/refs/HistoEx2.pyref | 22 ++++++++---------
 GaudiUtils/src/Lib/HistoDump.cpp          | 29 +++++++++--------------
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/GaudiAlg/tests/qmtest/refs/HistoEx2.pyref b/GaudiAlg/tests/qmtest/refs/HistoEx2.pyref
index 6a60d9f559..6d0fea43d3 100644
--- a/GaudiAlg/tests/qmtest/refs/HistoEx2.pyref
+++ b/GaudiAlg/tests/qmtest/refs/HistoEx2.pyref
@@ -158,27 +158,27 @@ RndmGenSvc           INFO Using Random engine:HepRndm::Engine<CLHEP::RanluxEngin
  | id                        :  x vs y    (profile)                          |
 
 
-          4   ++----+----+----+----+----+----+----+----+----+----+
+          1   ++----+----+----+----+----+----+----+----+----+----+
               |.    .    .    .    .    |    .    .    .    .    |
               |.    .    .    .    .    |    .    .    .    .    |
               |.    .    .    .    .    |    .    .    .    .    |
               |.    .    .    .    .    |    .    .    .    .    |
-        1.5   +.........................+........................+
+       0.75   +.........................+........................+
               |.    .    .    .    .    |    .    .    .    .    |
               |.    .    .    .    .    |    .    .    .    .    |
-          0 I-+IIII**I***+I-*II-II-*--*III--**-*-*I-**III-I*II*I-->*
-            * |****I.*III***I******.**I*****I.*I*I**II*****I**I**|
-         -1   +.........................+................I......I+
               |.    .    .    .    .    |    .    .    .    .    |
               |.    .    .    .    .    |    .    .    .    .    |
+        0.5   +.........................+........................+
               |.    .    .    .    .    |    .    .    .    .    |
               |.    .    .    .    .    |    .    .    .    .    |
-       -3.5   +.........................+........................+
-              |.    .    .    .    .    |    .    .    .    .    |
-              |.    .    .    .    .    |    .    .    .    .    |
-              |.    .    .    .    .    |    .    .    .    .    |
-              |.    .    .    .    .    |    .    .    .    .    |
-         -6   ++----+----+----+----+----+----+----+----+----+----+
+              |.    I    .    .    .    |    .    .    .    .    |
+              |.    I    .    .    .    |    .    .    .    .    |
+       0.25   +.....I...................+.....................I..+
+              |.   I*    .    .    .    |    .    . I  .   I. I  |
+              |.I  II II .    .    I    |    I I  . I  .   I. I  |
+              |.I  II III.I I .    I  I |   I* I I. II .   I.II  | I
+              |III *I III.I I I I  *  I |I  II * I. *I .   *.I*  | *
+          0 I-+IIIII+I***+I-*II-II-I--*III--*I-I-*I-I*III-IIIIII-->I
             U
             N                                                      O
             D                                                      V
diff --git a/GaudiUtils/src/Lib/HistoDump.cpp b/GaudiUtils/src/Lib/HistoDump.cpp
index a7c2679929..d66e87a1df 100644
--- a/GaudiUtils/src/Lib/HistoDump.cpp
+++ b/GaudiUtils/src/Lib/HistoDump.cpp
@@ -12,9 +12,6 @@
 // disable icc remark #2259: non-pointer conversion from "X" to "Y" may lose significant bits
 //   TODO: To be removed, since it comes from ROOT TMathBase.h
 #  pragma warning( disable : 2259 )
-// disable icc remark #1572: floating-point equality and inequality comparisons are unreliable
-//   The comparison are meant
-#  pragma warning( disable : 1572 )
 #endif
 #ifdef WIN32
 // Disable warning
@@ -44,6 +41,12 @@
 
 // ============================================================================
 namespace {
+
+  // idea coming from The art of computer programming by Knuth
+  constexpr bool essentiallyEqual( double const a, double const b ) {
+    return std::abs( a - b ) <= std::min( std::abs( a ), std::abs( b ) ) * std::numeric_limits<double>::epsilon();
+  }
+
   // ==========================================================================
   /** @struct Histo
    *  helper structure to keep the representation of the histogram
@@ -255,10 +258,10 @@ namespace {
    *  @date 2009-09-19
    */
   std::pair<double, int> decompose( double v ) {
-    if ( 0 == v ) {
+    if ( abs( v ) < std::numeric_limits<double>::epsilon() ) {
       return { 0.0, 0 };
     } // RETURN
-    else if ( 1 == v ) {
+    else if ( essentiallyEqual( 1.0, v ) ) {
       return { 1.0, 0 };
     } // RETURN
     else if ( 0 > v ) {
@@ -302,12 +305,7 @@ namespace {
    *  @date 2009-09-19
    */
   inline double rValMax( double v ) {
-    if ( 0 == v ) {
-      return 0;
-    } // RETURN
-    else if ( 0 > v ) {
-      return -1 * rValMin( -v );
-    } // RETURN
+    if ( 0 > v ) { return -1 * rValMin( -v ); } // RETURN
     // decompose the double value into decimal significand and mantissa
     std::pair<double, int> r = decompose( v );
     //
@@ -321,12 +319,7 @@ namespace {
    *  @date 2009-09-19
    */
   inline double rValMin( double v ) {
-    if ( 0 == v ) {
-      return 0;
-    } // RETURN
-    else if ( 0 > v ) {
-      return -1 * rValMax( -v );
-    } // RETURN
+    if ( 0 > v ) { return -1 * rValMax( -v ); } // RETURN
     // decompose the double value into decimal significand and mantissa
     std::pair<double, int> r = decompose( v );
     const double           f = std::floor( 20 * r.first ) / 2; // - 1 ;
@@ -392,7 +385,7 @@ namespace {
     double yMax = std::max( rValMax( histo.maxY( errors ) ), 0.0 );
     double yMin = std::min( rValMin( histo.minY( errors ) ), 0.0 );
 
-    if ( yMin == yMax ) { yMax = yMin + 1; }
+    if ( essentiallyEqual( yMin, yMax ) ) { yMax = yMin + 1; }
     /// try to define the proper "Y-binning"
     std::pair<double, int> r   = decompose( yMax - yMin );
     double                 _ny = std::ceil( 10 * r.first ); //   1 <= ny < 10
-- 
GitLab