ConstantsComparator: reduced code duplications, fixed a potentially div by 0, and silenced a warning

ConstantsComparator: The old code used `abs(x-y)/abs(x+y)<= epsilon` when comparing two numbers for equality (modulo relative precision). This is weird when x and y have different signs and potentially even causes a div by 0 whenever x=-y. Moreover the templating was awkward, causing a lot of code duplications and a Warning with clang.
Distribution[WithReward]: replaced forward declarations of ConstantsComparator by actual includes as the forward declarations caused some ambiguity regarding the template parameters.

src/storm/storage/Distribution.h

@@ -6,12 +6,10 @@
 #include <boost/container/flat_map.hpp>
 
 #include "storm/storage/sparse/StateType.h"
+#include "storm/utility/ConstantsComparator.h"
 
 namespace storm {
-    namespace utility {
-        template <typename ValueType>
-        class ConstantsComparator;
-    }
+
     
     namespace storage {
         

src/storm/storage/DistributionWithReward.h

@@ -3,12 +3,9 @@
 #include "storm/storage/Distribution.h"
 
 #include "storm/utility/constants.h"
+#include "storm/utility/ConstantsComparator.h"
 
 namespace storm {
-    namespace utility {
-        template <typename ValueType>
-        class ConstantsComparator;
-    }
 
     namespace storage {
         

src/storm/utility/ConstantsComparator.cpp

@@ -1,160 +1,99 @@
 #include "storm/utility/ConstantsComparator.h"
 
-#include <cstdlib>
-#include <cmath>
 #include "storm/storage/sparse/StateType.h"
 
-#include "storm/utility/constants.h"
 #include "storm/settings/SettingsManager.h"
 #include "storm/settings/modules/GeneralSettings.h"
+#include "storm/utility/NumberTraits.h"
+#include "storm/utility/constants.h"
 
 namespace storm {
     namespace utility {
-        template<typename ValueType>
-        bool ConstantsComparator<ValueType>::isOne(ValueType const& value) const {
-            return storm::utility::isOne(value);
+
+        template<typename ValueType, typename Enable>
+        bool ConstantsComparator<ValueType, Enable>::isOne(ValueType const& value) const {
+            return isEqual(value, storm::utility::one<ValueType>());
         }
 
-        template<typename ValueType>
-        bool ConstantsComparator<ValueType>::isZero(ValueType const& value) const {
-            return storm::utility::isZero(value);
+        template<typename ValueType, typename Enable>
+        bool ConstantsComparator<ValueType, Enable>::isZero(ValueType const& value) const {
+            return isEqual(value, storm::utility::zero<ValueType>());
         }
 
-        template<typename ValueType>
-        bool ConstantsComparator<ValueType>::isEqual(ValueType const& value1, ValueType const& value2) const {
+        template<typename ValueType, typename Enable>
+        bool ConstantsComparator<ValueType, Enable>::isEqual(ValueType const& value1, ValueType const& value2) const {
             return value1 == value2;
         }
 
-        template<typename ValueType>
-        bool ConstantsComparator<ValueType>::isConstant(ValueType const& value) const {
+        template<typename ValueType, typename Enable>
+        bool ConstantsComparator<ValueType, Enable>::isConstant(ValueType const& value) const {
             return storm::utility::isConstant(value);
         }
 
-        template<typename ValueType>
-        bool ConstantsComparator<ValueType>::isInfinity(ValueType const&) const {
-            return false;
-        }
-        
-        template<typename ValueType>
-        bool ConstantsComparator<ValueType>::isLess(ValueType const& value1, ValueType const& value2) const {
+        template<typename ValueType, typename Enable>
+        bool ConstantsComparator<ValueType, Enable>::isLess(ValueType const& value1, ValueType const& value2) const {
             return value1 < value2;
         }
 
-        ConstantsComparator<float>::ConstantsComparator() : precision(static_cast<float>(storm::settings::getModule<storm::settings::modules::GeneralSettings>().getPrecision())) {
-            // Intentionally left empty.
-        }
-        
-        ConstantsComparator<float>::ConstantsComparator(float precision) : precision(precision) {
-            // Intentionally left empty.
-        }
-        
-        bool ConstantsComparator<float>::isOne(float const& value) const {
-            return std::abs(value - one<float>()) <= precision;
-        }
-        
-        bool ConstantsComparator<float>::isZero(float const& value) const {
-            return std::abs(value) <= precision;
-        }
-        
-        bool ConstantsComparator<float>::isEqual(float const& value1, float const& value2) const {
-            return std::abs(value1 - value2) <= precision;
-        }
-        
-        bool ConstantsComparator<float>::isConstant(float const&) const {
-            return true;
-        }
-        
-        bool ConstantsComparator<float>::isInfinity(float const& value) const {
-            return value == storm::utility::infinity<float>();
-        }
-        
-        bool ConstantsComparator<float>::isLess(float const& value1, float const& value2) const {
-            return std::abs(value1 - value2) < precision;
-        }
-        
-        ConstantsComparator<double>::ConstantsComparator() : precision(storm::settings::getModule<storm::settings::modules::GeneralSettings>().getPrecision()), relative(false) {
-            // Intentionally left empty.
-        }
-        
-        ConstantsComparator<double>::ConstantsComparator(double precision, bool relative) : precision(precision), relative(relative) {
-            // Intentionally left empty.
-        }
-        
-        bool ConstantsComparator<double>::isOne(double const& value) const {
-            return std::abs(value - one<double>()) <= precision;
-        }
-        
-        bool ConstantsComparator<double>::isZero(double const& value) const {
-            return std::abs(value) <= precision;
-        }
-        
-        bool ConstantsComparator<double>::isInfinity(double const& value) const {
-            return value == infinity<double>();
-        }
-        
-        bool ConstantsComparator<double>::isEqual(double const& value1, double const& value2) const {
-            if (relative) {
-                return value1 == value2 || std::abs(value1 - value2)/std::abs(value1 + value2) <= precision;
-            } else {
-                return std::abs(value1 - value2) <= precision;
-            }
-        }
-        
-        bool ConstantsComparator<double>::isConstant(double const&) const {
-            return true;
-        }
-        
-        bool ConstantsComparator<double>::isLess(double const& value1, double const& value2) const {
-            return value1 < value2 - precision;
-        }
-        
-        ConstantsComparator<storm::RationalNumber>::ConstantsComparator() : precision(storm::utility::zero<storm::RationalNumber>()), relative(false) {
-            // Intentionally left empty.
+        template<typename ValueType>
+        ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::ConstantsComparator()
+            : ConstantsComparator(
+                  storm::NumberTraits<ValueType>::IsExact
+                      ? storm::utility::zero<ValueType>()
+                      : storm::utility::convertNumber<ValueType>(storm::settings::getModule<storm::settings::modules::GeneralSettings>().getPrecision())) {
+            // Intentionally left empty
         }
 
-        ConstantsComparator<storm::RationalNumber>::ConstantsComparator(storm::RationalNumber precision, bool relative) : precision(precision), relative(relative) {
-            // Intentionally left empty.
+        template<typename ValueType>
+        ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::ConstantsComparator(ValueType const& precision, bool const& relative)
+            : precision(precision), relative(relative) {
+            // Intentionally left empty
         }
 
-        bool ConstantsComparator<storm::RationalNumber>::isOne(storm::RationalNumber const& value) const {
-            if (storm::utility::isZero(precision)) {
-                return storm::utility::isOne(value);
-            }
-            return storm::utility::abs(storm::RationalNumber(value - one<storm::RationalNumber>())) <= precision;
+        template<typename ValueType>
+        bool ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::isOne(ValueType const& value) const {
+            return isEqual(value, storm::utility::one<ValueType>());
         }
 
-        bool ConstantsComparator<storm::RationalNumber>::isZero(storm::RationalNumber const& value) const {
-            if (storm::utility::isZero(precision)) {
-                return storm::utility::isZero(value);
-            }
-            return storm::utility::abs(value) <= precision;
+        template<typename ValueType>
+        bool ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::isZero(ValueType const& value) const {
+            return isEqual(value, storm::utility::zero<ValueType>());
         }
 
-        bool ConstantsComparator<storm::RationalNumber>::isEqual(storm::RationalNumber const& value1, storm::RationalNumber const& value2) const {
-            if (storm::utility::isZero(precision)) {
-                return value1 == value2;
-            }
-            
-            if (relative) {
-                return value1 == value2 || storm::utility::abs(storm::RationalNumber(value1 - value2))/storm::utility::abs(storm::RationalNumber(value1 + value2)) <= precision;
+        template<typename ValueType>
+        bool ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::isEqual(ValueType const& value1, ValueType const& value2) const {
+            if (value1 == value2) {
+                return true;
+            } else if (storm::utility::isZero(precision)) {
+                return false;
             } else {
-                return storm::utility::abs(storm::RationalNumber(value1 - value2)) <= precision;
+                ValueType absDiff = storm::utility::abs<ValueType>(value1 - value2);
+                if (relative) {
+                    return absDiff <= precision * (storm::utility::abs(value1) + storm::utility::abs(value2));
+                } else {
+                    return absDiff <= precision;
+                }
             }
         }
 
-        bool ConstantsComparator<storm::RationalNumber>::isConstant(storm::RationalNumber const&) const {
-            return true;
+        template<typename ValueType>
+        bool ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::isConstant(ValueType const& value) const {
+            return storm::utility::isConstant(value);
         }
 
-        bool ConstantsComparator<storm::RationalNumber>::isInfinity(storm::RationalNumber const&) const {
-            return false;
+        template<typename ValueType>
+        bool ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::isInfinity(ValueType const& value) const {
+            return storm::utility::isInfinity(value);
         }
 
-        bool ConstantsComparator<storm::RationalNumber>::isLess(storm::RationalNumber const& value1, storm::RationalNumber const& value2) const {
+        template<typename ValueType>
+        bool ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>>::isLess(ValueType const& value1, ValueType const& value2) const {
             return value1 < value2 - precision;
         }
 
         // Explicit instantiations.
+        template class ConstantsComparator<double>;
+        template class ConstantsComparator<float>;
         template class ConstantsComparator<int>;
         template class ConstantsComparator<storm::storage::sparse::state_type>;
 
@@ -171,5 +110,5 @@ namespace storm {
         template class ConstantsComparator<Polynomial>;
         template class ConstantsComparator<Interval>;
 #endif
-    }
-}
+    }  // namespace utility
+}  // namespace storm

src/storm/utility/ConstantsComparator.h

@@ -1,108 +1,43 @@
-#ifndef STORM_UTILITY_CONSTANTSCOMPARATOR_H_
-#define STORM_UTILITY_CONSTANTSCOMPARATOR_H_
+#pragma once
 
-#include "storm/adapters/RationalFunctionAdapter.h"
+#include <type_traits>
+#include "storm/adapters/RationalNumberAdapter.h"
 
 namespace storm {
     namespace utility {
+
         // A class that can be used for comparing constants.
-        template<typename ValueType>
+        template<typename ValueType, typename Enable = void>
         class ConstantsComparator {
-        public:
-            // This needs to be in here, otherwise the template specializations are not used properly.
+           public:
             ConstantsComparator() = default;
-            
             bool isOne(ValueType const& value) const;
-            
             bool isZero(ValueType const& value) const;
-            
             bool isEqual(ValueType const& value1, ValueType const& value2) const;
-            
             bool isConstant(ValueType const& value) const;
-            
-            bool isInfinity(ValueType const& value) const;
-            
             bool isLess(ValueType const& value1, ValueType const& value2) const;
         };
 
-        // For floats we specialize this class and consider the comparison modulo some predefined precision.
-        template<>
-        class ConstantsComparator<float> {
-        public:
-            ConstantsComparator();
-            
-            ConstantsComparator(float precision);
-            
-            bool isOne(float const& value) const;
-            
-            bool isZero(float const& value) const;
-            
-            bool isEqual(float const& value1, float const& value2) const;
-            
-            bool isConstant(float const& value) const;
-            
-            bool isInfinity(float const& value) const;
-
-            bool isLess(float const& value1, float const& value2) const;
-            
-        private:
-            // The precision used for comparisons.
-            float precision;
-        };
-        
-        // For doubles we specialize this class and consider the comparison modulo some predefined precision.
-        template<>
-        class ConstantsComparator<double> {
-        public:
-            ConstantsComparator();
-            
-            ConstantsComparator(double precision, bool relative = false);
-            
-            bool isOne(double const& value) const;
-            
-            bool isZero(double const& value) const;
-            
-            bool isInfinity(double const& value) const;
-            
-            bool isEqual(double const& value1, double const& value2) const;
-            
-            bool isConstant(double const& value) const;
-            
-            bool isLess(double const& value1, double const& value2) const;
-            
-        private:
-            // The precision used for comparisons.
-            double precision;
-            
-            // Whether to use relative comparison for equality.
-            bool relative;
-        };
+        // Specialization for numbers where there can be a precision
+        template<typename ValueType>
+        using ConstantsComparatorEnablePrecision =
+            typename std::enable_if_t<std::is_same<ValueType, double>::value || std::is_same<ValueType, storm::RationalNumber>::value>;
 
-        // For rational numbers we specialize this class and consider the comparison modulo some predefined precision.
-        template<>
-        class ConstantsComparator<storm::RationalNumber> {
-        public:
+        template<typename ValueType>
+        class ConstantsComparator<ValueType, ConstantsComparatorEnablePrecision<ValueType>> {
+           public:
             ConstantsComparator();
+            ConstantsComparator(ValueType const& precision, bool const& relative = false);
+            bool isOne(ValueType const& value) const;
+            bool isZero(ValueType const& value) const;
+            bool isEqual(ValueType const& value1, ValueType const& value2) const;
+            bool isConstant(ValueType const& value) const;
+            bool isInfinity(ValueType const& value) const;
+            bool isLess(ValueType const& value1, ValueType const& value2) const;
 
-            ConstantsComparator(storm::RationalNumber precision, bool relative);
-            
-            bool isOne(storm::RationalNumber const& value) const;
-            
-            bool isZero(storm::RationalNumber const& value) const;
-            
-            bool isEqual(storm::RationalNumber const& value1, storm::RationalNumber const& value2) const;
-            
-            bool isConstant(storm::RationalNumber const& value) const;
-            
-            bool isInfinity(storm::RationalNumber const& value) const;
-            
-            bool isLess(storm::RationalNumber const& value1, storm::RationalNumber const& value2) const;
-            
-        private:
-            storm::RationalNumber precision;
+           private:
+            ValueType precision;
             bool relative;
         };
-    }
-}
-
-#endif /* STORM_UTILITY_CONSTANTSCOMPARATOR_H_ */
+    }  // namespace utility
+}  // namespace storm