Fixed a rounding bug in the Matrix GetHashCode that would cause matri…

…ces that were deemed equal to not have the same hash code
kevmoo · Apr 26, 2010 · 9bc97fa · 9bc97fa
1 parent 592a82b
commit 9bc97fa
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 6 deletions.
diff --git a/Skills/Numerics/Matrix.cs b/Skills/Numerics/Matrix.cs
@@ -9,6 +9,10 @@ namespace Moserware.Numerics
     /// </summary>    
     internal class Matrix
     {
+        // Anything smaller than this will be assumed to be rounding error in terms of equality matching
+        private const int FractionalDigitsToRoundTo = 10;
+        private static readonly double ErrorTolerance = Math.Pow(0.1, FractionalDigitsToRoundTo); // e.g. 1/10^10
+
         protected double[][] _MatrixRowValues;
         // Note: some properties like Determinant, Inverse, etc are properties instead
         // of methods to make the syntax look nicer even though this sort of goes against
@@ -386,9 +390,7 @@ private double GetCofactor(int rowToRemove, int columnToRemove)
             {
                 return false;
             }
-
-            const double errorTolerance = 0.0000000000001;
-
+
             for (int currentRow = 0; currentRow < a.Rows; currentRow++)
             {
                 for (int currentColumn = 0; currentColumn < a.Columns; currentColumn++)
@@ -397,7 +399,7 @@ private double GetCofactor(int rowToRemove, int columnToRemove)
                         Math.Abs(a._MatrixRowValues[currentRow][currentColumn] -
                                  b._MatrixRowValues[currentRow][currentColumn]);
 
-                    if (delta > errorTolerance)
+                    if (delta > ErrorTolerance)
                     {
                         return false;
                     }
@@ -426,7 +428,9 @@ public override int GetHashCode()
 
                     for (int currentColumn = 0; currentColumn < Columns; currentColumn++)
                     {
-                        result += multiplier*_MatrixRowValues[currentRow][currentColumn];
+                        double cellValue = _MatrixRowValues[currentRow][currentColumn];
+                        double roundedValue = Math.Round(cellValue, FractionalDigitsToRoundTo);
+                        result += multiplier*roundedValue;
                     }
                 }
             }
@@ -443,7 +447,7 @@ public override int GetHashCode()
             int hashCode = BitConverter.ToInt32(finalBytes, 0);
             return hashCode;
         }
-
+        
         public override bool Equals(object obj)
         {
             var other = obj as Matrix;

diff --git a/Skills/bin/Debug/Moserware.Skills.dll b/Skills/bin/Debug/Moserware.Skills.dll
diff --git a/Skills/bin/Debug/Moserware.Skills.pdb b/Skills/bin/Debug/Moserware.Skills.pdb
diff --git a/Skills/bin/Release/Moserware.Skills.dll b/Skills/bin/Release/Moserware.Skills.dll
diff --git a/Skills/bin/Release/Moserware.Skills.pdb b/Skills/bin/Release/Moserware.Skills.pdb
diff --git a/UnitTests/Numerics/MatrixTests.cs b/UnitTests/Numerics/MatrixTests.cs
@@ -121,6 +121,16 @@ public void EqualsTest()
             Assert.AreEqual(d, f);
             Assert.AreEqual(d.GetHashCode(), f.GetHashCode());
 
+            // Test rounding (thanks to nsp on GitHub for finding this case)
+            var g = new SquareMatrix(1, 2.00000000000001,
+                                     3, 4);
+
+            var h = new SquareMatrix(1, 2,
+                                     3, 4);
+
+            Assert.IsTrue(g == h);
+            Assert.AreEqual(g, h);
+            Assert.AreEqual(g.GetHashCode(), h.GetHashCode());
         }
 
         [Test]