Resolve CoinRational warnings and add tests (#246)

Resolve CoinRational warnings related to int64_t and add tests
coin-or · Feb 20, 2025 · 5d621c0 · 5d621c0
1 parent 328760e
commit 5d621c0
Show file tree

Hide file tree

Showing 7 changed files with 188 additions and 4 deletions.
diff --git a/MSVisualStudio/v17/CoinUtilsUnitTest/CoinUtilsUnitTest.vcxproj b/MSVisualStudio/v17/CoinUtilsUnitTest/CoinUtilsUnitTest.vcxproj
@@ -157,6 +157,7 @@
     <ClCompile Include="..\..\..\test\CoinMpsIOTest.cpp" />
     <ClCompile Include="..\..\..\test\CoinPackedMatrixTest.cpp" />
     <ClCompile Include="..\..\..\test\CoinPackedVectorTest.cpp" />
+    <ClCompile Include="..\..\..\test\CoinRationalTest.cpp" />
     <ClCompile Include="..\..\..\test\CoinShallowPackedVectorTest.cpp" />
     <ClCompile Include="..\..\..\test\unitTest.cpp" />
   </ItemGroup>

diff --git a/src/CoinRational.cpp b/src/CoinRational.cpp
@@ -29,8 +29,8 @@ bool CoinRational::nearestRational_(double val, double maxdelta, int64_t maxdnom
 {
   double intpart;
   if (floor(val)==val) {
-    numerator_ = val;
-    denominator_ = 1.0;
+    numerator_ = (int64_t) val;
+    denominator_ = 1;
     return true;
   }
   double fracpart = fabs(modf(val, &intpart));
@@ -83,7 +83,7 @@ bool CoinRational::nearestRational_(double val, double maxdelta, int64_t maxdnom
     assert(inaccuracy <= maxdelta);
   }
 #endif
-  numerator_ += std::abs(intpart) * denominator_;
+  numerator_ += ((int64_t) std::abs(intpart)) * denominator_;
   if (val < 0)
     numerator_ *= -1;
 #if DEBUG_X > 1

diff --git a/src/CoinRational.hpp b/src/CoinRational.hpp
@@ -40,6 +40,14 @@ class COINUTILSLIB_EXPORT CoinRational
   bool nearestRational_(double val, double maxdelta, int64_t maxdnom);
 };
 
+//#############################################################################
+/** A function that tests the methods in the class. The
+    only reason for it not to be a member method is that this way it doesn't
+    have to be compiled into the library. And that's a gain, because the
+    library should be compiled with optimization on, but this method should be
+    compiled with debugging. */
+void CoinRationalUnitTest();
+
 #endif
 
 /* vi: softtabstop=2 shiftwidth=2 expandtab tabstop=2

diff --git a/test/CoinRationalTest.cpp b/test/CoinRationalTest.cpp
@@ -0,0 +1,165 @@
+// This code is licensed under the terms of the Eclipse Public License (EPL).
+
+#ifdef NDEBUG
+#undef NDEBUG
+#endif
+
+#include "CoinUtilsConfig.h"
+
+#ifdef COINUTILS_HAS_STDINT_H
+#include <stdint.h>
+#endif
+
+#include <cassert>
+#include <iostream>
+#include "CoinRational.hpp"
+
+void
+CoinRationalUnitTest()
+{
+
+    // Test default constructor
+    {
+        CoinRational a; // 0/1
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 0);
+        assert(a.getDenominator() == 1);
+    }
+
+    // Requires int64_t
+    // Test constructor with assignment
+    {
+        CoinRational a(4294967295, 4294967296);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 4294967295);
+        assert(a.getDenominator() == 4294967296);
+    }
+
+    // Requires int64_t
+    // Test constructor with assignment
+    {
+        CoinRational a(9223372036854775807, 4294967299);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 9223372036854775807);
+        assert(a.getDenominator() == 4294967299);
+    }
+
+    // Requires int64_t
+    // Test constructor with nearestRational calls
+    {
+        CoinRational a(2147483699.5, 0.00001, 4294967299);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 4294967399);
+        assert(a.getDenominator() == 2);
+    }
+
+    // Test constructor with nearestRational calls
+    {
+        CoinRational a(-3.0, 0.0001, 100);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == -3);
+        assert(a.getDenominator() == 1);
+    }
+
+    {
+        CoinRational a(3.0, 0.0001, 100);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 3);
+        assert(a.getDenominator() == 1);
+    }
+
+    {
+        // return 0/1 if best is more than maxdelta tolerance
+        CoinRational a(0.367879441, 0.0001, 100); // 1/e
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 32);
+        assert(a.getDenominator() == 87);
+    }
+
+    {
+        CoinRational a(10.367879441, 0.0001, 100); // 10 + 1/e
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 902);
+        assert(a.getDenominator() == 87);
+    }
+
+    {
+        // return 0/1 if best is more than maxdelta tolerance
+        CoinRational a(0.367879441, 0.000001, 100); // 1/e
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 0);
+        assert(a.getDenominator() == 1);
+    }
+
+    {
+        CoinRational a(3.0 / 7.0, 0.0001, 100);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 3);
+        assert(a.getDenominator() == 7);
+    }
+
+    {
+        CoinRational a(7.0 / 3.0, 0.0001, 100);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 7);
+        assert(a.getDenominator() == 3);
+    }
+
+    {
+        double sqrt13 = sqrt(13);
+        CoinRational a(sqrt13, 0.01, 20);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 18);
+        assert(a.getDenominator() == 5);
+    }
+
+    {
+        double sqrt13 = sqrt(13);
+        CoinRational a(sqrt13, 0.002, 30);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 101);
+        assert(a.getDenominator() == 28);
+    }
+
+    {
+        CoinRational a(0.25, 0.1, 3);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 1);
+        assert(a.getDenominator() == 3);
+    }
+
+    {
+        CoinRational a(0.605551, 0.003, 30);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 17);
+        assert(a.getDenominator() == 28);
+    }
+
+    {
+        CoinRational a(0.605551, 0.001, 30);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 20);
+        assert(a.getDenominator() == 33); // oops, should be at most 30.
+    }
+
+    {
+        CoinRational a(0.58496250072, 0.00001, 253);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 179);
+        assert(a.getDenominator() == 306);  // oops, should be at most 253. Expected 148/253, but this is apparently on purpose.
+    }
+
+    {
+        CoinRational a(19.0/11.0, 0.02, 10);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == 12);
+        assert(a.getDenominator() == 7);
+    }
+
+    {
+        CoinRational a(-19.0 / 11.0, 0.02, 10);
+        std::cout << "Testing " << a.getNumerator() << " / " << a.getDenominator() << std::endl;
+        assert(a.getNumerator() == -12);
+        assert(a.getDenominator() == 7);
+    }
+}
diff --git a/test/Makefile.am b/test/Makefile.am
@@ -20,6 +20,7 @@ unitTest_SOURCES = \
 	CoinMpsIOTest.cpp \
 	CoinPackedMatrixTest.cpp \
 	CoinPackedVectorTest.cpp \
+	CoinRationalTest.cpp \	
 	CoinShallowPackedVectorTest.cpp \
 	unitTest.cpp
 

diff --git a/test/Makefile.in b/test/Makefile.in
@@ -123,7 +123,7 @@ am_unitTest_OBJECTS = CoinLpIOTest.$(OBJEXT) \
 	CoinIndexedVectorTest.$(OBJEXT) \
 	CoinMessageHandlerTest.$(OBJEXT) CoinModelTest.$(OBJEXT) \
 	CoinMpsIOTest.$(OBJEXT) CoinPackedMatrixTest.$(OBJEXT) \
-	CoinPackedVectorTest.$(OBJEXT) \
+	CoinPackedVectorTest.$(OBJEXT) CoinRationalTest.$(OBJEXT) \
 	CoinShallowPackedVectorTest.$(OBJEXT) unitTest.$(OBJEXT)
 unitTest_OBJECTS = $(am_unitTest_OBJECTS)
 am__DEPENDENCIES_1 =
@@ -155,6 +155,7 @@ am__depfiles_remade = ./$(DEPDIR)/CoinDenseVectorTest.Po \
 	./$(DEPDIR)/CoinModelTest.Po ./$(DEPDIR)/CoinMpsIOTest.Po \
 	./$(DEPDIR)/CoinPackedMatrixTest.Po \
 	./$(DEPDIR)/CoinPackedVectorTest.Po \
+	./$(DEPDIR)/CoinRationalTest.Po \
 	./$(DEPDIR)/CoinShallowPackedVectorTest.Po \
 	./$(DEPDIR)/unitTest.Po
 am__mv = mv -f
@@ -361,6 +362,7 @@ unitTest_SOURCES = \
 	CoinMpsIOTest.cpp \
 	CoinPackedMatrixTest.cpp \
 	CoinPackedVectorTest.cpp \
+	CoinRationalTest.cpp \
 	CoinShallowPackedVectorTest.cpp \
 	unitTest.cpp
 
@@ -439,6 +441,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/CoinMpsIOTest.Po@am__quote@ # am--include-marker
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/CoinPackedMatrixTest.Po@am__quote@ # am--include-marker
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/CoinPackedVectorTest.Po@am__quote@ # am--include-marker
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/CoinRationalTest.Po@am__quote@ # am--include-marker
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/CoinShallowPackedVectorTest.Po@am__quote@ # am--include-marker
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/unitTest.Po@am__quote@ # am--include-marker
 
@@ -579,6 +582,7 @@ distclean: distclean-am
 	-rm -f ./$(DEPDIR)/CoinMpsIOTest.Po
 	-rm -f ./$(DEPDIR)/CoinPackedMatrixTest.Po
 	-rm -f ./$(DEPDIR)/CoinPackedVectorTest.Po
+	-rm -f ./$(DEPDIR)/CoinRationalTest.Po
 	-rm -f ./$(DEPDIR)/CoinShallowPackedVectorTest.Po
 	-rm -f ./$(DEPDIR)/unitTest.Po
 	-rm -f Makefile
@@ -635,6 +639,7 @@ maintainer-clean: maintainer-clean-am
 	-rm -f ./$(DEPDIR)/CoinMpsIOTest.Po
 	-rm -f ./$(DEPDIR)/CoinPackedMatrixTest.Po
 	-rm -f ./$(DEPDIR)/CoinPackedVectorTest.Po
+	-rm -f ./$(DEPDIR)/CoinRationalTest.Po
 	-rm -f ./$(DEPDIR)/CoinShallowPackedVectorTest.Po
 	-rm -f ./$(DEPDIR)/unitTest.Po
 	-rm -f Makefile

diff --git a/test/unitTest.cpp b/test/unitTest.cpp
@@ -16,6 +16,7 @@
 #include "CoinPragma.hpp"
 #include "CoinFinite.hpp"
 #include "CoinError.hpp"
+#include "CoinRational.hpp"
 #include "CoinHelperFunctions.hpp"
 #include "CoinSort.hpp"
 #include "CoinShallowPackedVector.hpp"
@@ -189,6 +190,9 @@ int main (int argc, const char *argv[])
   testingMessage( "Testing CoinError\n" );
   CoinErrorUnitTest();
 
+  testingMessage("Testing CoinRational\n");
+  CoinRationalUnitTest();
+
   testingMessage( "Testing CoinShallowPackedVector\n" );
   CoinShallowPackedVectorUnitTest();