LA: More consistent range checking in MatrixStorage.Clear and related routines #284

src/Numerics/LinearAlgebra/Matrix.cs

@@ -158,7 +158,7 @@ namespace MathNet.Numerics.LinearAlgebra
                 throw new ArgumentOutOfRangeException("rowIndex");
             }
 
-            Storage.Clear(rowIndex, 1, 0, ColumnCount);
+            Storage.ClearUnchecked(rowIndex, 1, 0, ColumnCount);
         }
 
         /// <summary>
@@ -171,7 +171,7 @@ namespace MathNet.Numerics.LinearAlgebra
                 throw new ArgumentOutOfRangeException("columnIndex");
             }
 
-            Storage.Clear(0, RowCount, columnIndex, 1);
+            Storage.ClearUnchecked(0, RowCount, columnIndex, 1);
         }
 
         /// <summary>
@@ -179,19 +179,6 @@ namespace MathNet.Numerics.LinearAlgebra
         /// </summary>
         public void ClearRows(params int[] rowIndices)
         {
-            if (rowIndices.Length == 0)
-            {
-                return;
-            }
-
-            for (int k = 0; k < rowIndices.Length; k++)
-            {
-                if (rowIndices[k] < 0 || rowIndices[k] >= RowCount)
-                {
-                    throw new ArgumentOutOfRangeException("rowIndices");
-                }
-            }
-
             Storage.ClearRows(rowIndices);
         }
 
@@ -200,43 +187,14 @@ namespace MathNet.Numerics.LinearAlgebra
         /// </summary>
         public void ClearColumns(params int[] columnIndices)
         {
-            if (columnIndices.Length == 0)
-            {
-                return;
-            }
-
-            for (int k = 0; k < columnIndices.Length; k++)
-            {
-                if (columnIndices[k] < 0 || columnIndices[k] >= ColumnCount)
-                {
-                    throw new ArgumentOutOfRangeException("columnIndices");
-                }
-            }
-
             Storage.ClearColumns(columnIndices);
         }
 
         /// <summary>
-        /// Sets all values of a submatrix to zero.
+        /// Sets all values of a sub-matrix to zero.
         /// </summary>
         public void ClearSubMatrix(int rowIndex, int rowCount, int columnIndex, int columnCount)
         {
-            if (rowCount < 1 || columnCount < 1)
-            {
-                // nothing to do (but no need to fail either)
-                return;
-            }
-
-            if (rowIndex + rowCount > RowCount || rowIndex < 0)
-            {
-                throw new ArgumentOutOfRangeException("rowIndex");
-            }
-
-            if (columnIndex + columnCount > ColumnCount || columnIndex < 0)
-            {
-                throw new ArgumentOutOfRangeException("columnIndex");
-            }
-
             Storage.Clear(rowIndex, rowCount, columnIndex, columnCount);
         }
 

src/Numerics/LinearAlgebra/Storage/DenseColumnMajorMatrixStorage.cs

@@ -108,16 +108,18 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             Data[(column*RowCount) + row] = value;
         }
 
+        // CLEARING
+
         public override void Clear()
         {
             Array.Clear(Data, 0, Data.Length);
         }
 
-        public override void Clear(int rowIndex, int rowCount, int columnIndex, int columnCount)
+        internal override void ClearUnchecked(int rowIndex, int rowCount, int columnIndex, int columnCount)
         {
             if (rowIndex == 0 && columnIndex == 0 && rowCount == RowCount && columnCount == ColumnCount)
             {
-                Clear();
+                Array.Clear(Data, 0, Data.Length);
                 return;
             }
 
@@ -127,7 +129,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             }
         }
 
-        public override void ClearRows(int[] rowIndices)
+        internal override void ClearRowsUnchecked(int[] rowIndices)
         {
             for (var j = 0; j < ColumnCount; j++)
             {
@@ -139,7 +141,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             }
         }
 
-        public override void ClearColumns(int[] columnIndices)
+        internal override void ClearColumnsUnchecked(int[] columnIndices)
         {
             for (int k = 0; k < columnIndices.Length; k++)
             {

src/Numerics/LinearAlgebra/Storage/DiagonalMatrixStorage.cs

@@ -115,37 +115,6 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             }
         }
 
-        public override void Clear()
-        {
-            Array.Clear(Data, 0, Data.Length);
-        }
-
-        public override void Clear(int rowIndex, int rowCount, int columnIndex, int columnCount)
-        {
-            var beginInclusive = Math.Max(rowIndex, columnIndex);
-            var endExclusive = Math.Min(rowIndex + rowCount, columnIndex + columnCount);
-            if (endExclusive > beginInclusive)
-            {
-                Array.Clear(Data, beginInclusive, endExclusive - beginInclusive);
-            }
-        }
-
-        public override void ClearRows(int[] rowIndices)
-        {
-            for (int i = 0; i < rowIndices.Length; i++)
-            {
-                Data[rowIndices[i]] = Zero;
-            }
-        }
-
-        public override void ClearColumns(int[] columnIndices)
-        {
-            for (int i = 0; i < columnIndices.Length; i++)
-            {
-                Data[columnIndices[i]] = Zero;
-            }
-        }
-
         /// <summary>
         /// Indicates whether the current object is equal to another object of the same type.
         /// </summary>
@@ -204,6 +173,39 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             return hash;
         }
 
+        // CLEARING
+
+        public override void Clear()
+        {
+            Array.Clear(Data, 0, Data.Length);
+        }
+
+        internal override void ClearUnchecked(int rowIndex, int rowCount, int columnIndex, int columnCount)
+        {
+            var beginInclusive = Math.Max(rowIndex, columnIndex);
+            var endExclusive = Math.Min(rowIndex + rowCount, columnIndex + columnCount);
+            if (endExclusive > beginInclusive)
+            {
+                Array.Clear(Data, beginInclusive, endExclusive - beginInclusive);
+            }
+        }
+
+        internal override void ClearRowsUnchecked(int[] rowIndices)
+        {
+            for (int i = 0; i < rowIndices.Length; i++)
+            {
+                Data[rowIndices[i]] = Zero;
+            }
+        }
+
+        internal override void ClearColumnsUnchecked(int[] columnIndices)
+        {
+            for (int i = 0; i < columnIndices.Length; i++)
+            {
+                Data[columnIndices[i]] = Zero;
+            }
+        }
+
         // INITIALIZATION
 
         public static DiagonalMatrixStorage<T> OfMatrix(MatrixStorage<T> matrix)
@@ -381,7 +383,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
 
             if (existingData == ExistingData.Clear)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             if (sourceRowIndex == sourceColumnIndex)
@@ -422,7 +424,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
                     throw new NotSupportedException();
                 }
 
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
                 return;
             }
 
@@ -442,7 +444,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
         {
             if (existingData == ExistingData.Clear)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             if (sourceRowIndex > sourceColumnIndex && sourceColumnIndex + columnCount > sourceRowIndex)
@@ -790,7 +792,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
 
             if (existingData == ExistingData.Clear)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             if (sourceRowIndex == sourceColumnIndex)
@@ -870,7 +872,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             var processZeros = zeros == Zeros.Include || !Zero.Equals(f(0, 1, Zero));
             if (existingData == ExistingData.Clear && !processZeros)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             if (processZeros)

src/Numerics/LinearAlgebra/Storage/MatrixStorage.cs

@@ -128,52 +128,6 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
         /// <remarks>WARNING: This method is not thread safe. Use "lock" with it and be sure to avoid deadlocks.</remarks>
         public abstract void At(int row, int column, T value);
 
-        public virtual void Clear()
-        {
-            for (var i = 0; i < RowCount; i++)
-            {
-                for (var j = 0; j < ColumnCount; j++)
-                {
-                    At(i, j, Zero);
-                }
-            }
-        }
-
-        public virtual void Clear(int rowIndex, int rowCount, int columnIndex, int columnCount)
-        {
-            for (var i = rowIndex; i < rowIndex + rowCount; i++)
-            {
-                for (var j = columnIndex; j < columnIndex + columnCount; j++)
-                {
-                    At(i, j, Zero);
-                }
-            }
-        }
-
-        public virtual void ClearRows(int[] rowIndices)
-        {
-            for (var k = 0; k < rowIndices.Length; k++)
-            {
-                int row = rowIndices[k];
-                for (var j = 0; j < ColumnCount; j++)
-                {
-                    At(row, j, Zero);
-                }
-            }
-        }
-
-        public virtual void ClearColumns(int[] columnIndices)
-        {
-            for (var k = 0; k < columnIndices.Length; k++)
-            {
-                int column = columnIndices[k];
-                for (var i = 0; i < RowCount; i++)
-                {
-                    At(i, column, Zero);
-                }
-            }
-        }
-
         /// <summary>
         /// Indicates whether the current object is equal to another object of the same type.
         /// </summary>
@@ -250,6 +204,110 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             return hash;
         }
 
+        // CLEARING
+
+        public virtual void Clear()
+        {
+            for (var i = 0; i < RowCount; i++)
+            {
+                for (var j = 0; j < ColumnCount; j++)
+                {
+                    At(i, j, Zero);
+                }
+            }
+        }
+
+        public void Clear(int rowIndex, int rowCount, int columnIndex, int columnCount)
+        {
+            if (rowCount < 1 || columnCount < 1)
+            {
+                return;
+            }
+
+            if (rowIndex + rowCount > RowCount || rowIndex < 0)
+            {
+                throw new ArgumentOutOfRangeException("rowIndex");
+            }
+
+            if (columnIndex + columnCount > ColumnCount || columnIndex < 0)
+            {
+                throw new ArgumentOutOfRangeException("columnIndex");
+            }
+
+            ClearUnchecked(rowIndex, rowCount, columnIndex, columnCount);
+        }
+
+        internal virtual void ClearUnchecked(int rowIndex, int rowCount, int columnIndex, int columnCount)
+        {
+            for (var i = rowIndex; i < rowIndex + rowCount; i++)
+            {
+                for (var j = columnIndex; j < columnIndex + columnCount; j++)
+                {
+                    At(i, j, Zero);
+                }
+            }
+        }
+
+        public void ClearRows(int[] rowIndices)
+        {
+            if (rowIndices.Length == 0)
+            {
+                return;
+            }
+
+            for (int k = 0; k < rowIndices.Length; k++)
+            {
+                if (rowIndices[k] < 0 || rowIndices[k] >= RowCount)
+                {
+                    throw new ArgumentOutOfRangeException("rowIndices");
+                }
+            }
+
+            ClearRowsUnchecked(rowIndices);
+        }
+
+        public void ClearColumns(int[] columnIndices)
+        {
+            if (columnIndices.Length == 0)
+            {
+                return;
+            }
+
+            for (int k = 0; k < columnIndices.Length; k++)
+            {
+                if (columnIndices[k] < 0 || columnIndices[k] >= ColumnCount)
+                {
+                    throw new ArgumentOutOfRangeException("columnIndices");
+                }
+            }
+
+            ClearColumnsUnchecked(columnIndices);
+        }
+
+        internal virtual void ClearRowsUnchecked(int[] rowIndices)
+        {
+            for (var k = 0; k < rowIndices.Length; k++)
+            {
+                int row = rowIndices[k];
+                for (var j = 0; j < ColumnCount; j++)
+                {
+                    At(row, j, Zero);
+                }
+            }
+        }
+
+        internal virtual void ClearColumnsUnchecked(int[] columnIndices)
+        {
+            for (var k = 0; k < columnIndices.Length; k++)
+            {
+                int column = columnIndices[k];
+                for (var i = 0; i < RowCount; i++)
+                {
+                    At(i, column, Zero);
+                }
+            }
+        }
+
         // MATRIX COPY
 
         public void CopyTo(MatrixStorage<T> target, ExistingData existingData = ExistingData.Clear)

src/Numerics/LinearAlgebra/Storage/SparseCompressedRowMatrixStorage.cs

@@ -287,12 +287,83 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             MapInplace(x => x, Zeros.AllowSkip);
         }
 
+        /// <summary>
+        /// Indicates whether the current object is equal to another object of the same type.
+        /// </summary>
+        /// <param name="other">
+        /// An object to compare with this object.
+        /// </param>
+        /// <returns>
+        /// <c>true</c> if the current object is equal to the <paramref name="other"/> parameter; otherwise, <c>false</c>.
+        /// </returns>
+        public override bool Equals(MatrixStorage<T> other)
+        {
+            // Reject equality when the argument is null or has a different shape.
+            if (other == null || ColumnCount != other.ColumnCount || RowCount != other.RowCount)
+            {
+                return false;
+            }
+
+            // Accept if the argument is the same object as this.
+            if (ReferenceEquals(this, other))
+            {
+                return true;
+            }
+
+            var sparse = other as SparseCompressedRowMatrixStorage<T>;
+            if (sparse == null)
+            {
+                return base.Equals(other);
+            }
+
+            if (ValueCount != sparse.ValueCount)
+            {
+                // TODO: this is only correct if normalized
+                return false;
+            }
+
+            // If all else fails, perform element wise comparison.
+            for (var index = 0; index < ValueCount; index++)
+            {
+                // TODO: AlmostEquals
+                if (!Values[index].Equals(sparse.Values[index]) || ColumnIndices[index] != sparse.ColumnIndices[index])
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        /// <summary>
+        /// Returns a hash code for this instance.
+        /// </summary>
+        /// <returns>
+        /// A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table.
+        /// </returns>
+        public override int GetHashCode()
+        {
+            var values = Values;
+            var hashNum = Math.Min(ValueCount, 25);
+            int hash = 17;
+            unchecked
+            {
+                for (var i = 0; i < hashNum; i++)
+                {
+                    hash = hash*31 + values[i].GetHashCode();
+                }
+            }
+            return hash;
+        }
+
+        // CLEARING
+
         public override void Clear()
         {
             Array.Clear(RowPointers, 0, RowPointers.Length);
         }
 
-        public override void Clear(int rowIndex, int rowCount, int columnIndex, int columnCount)
+        internal override void ClearUnchecked(int rowIndex, int rowCount, int columnIndex, int columnCount)
         {
             if (rowIndex == 0 && columnIndex == 0 && rowCount == RowCount && columnCount == ColumnCount)
             {
@@ -346,7 +417,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             }
         }
 
-        public override void ClearRows(int[] rowIndices)
+        internal override void ClearRowsUnchecked(int[] rowIndices)
         {
             var rows = new bool[RowCount];
             for (int i = 0; i < rowIndices.Length; i++)
@@ -356,7 +427,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             MapIndexedInplace((i, j, x) => rows[i] ? Zero : x, Zeros.AllowSkip);
         }
 
-        public override void ClearColumns(int[] columnIndices)
+        internal override void ClearColumnsUnchecked(int[] columnIndices)
         {
             var columns = new bool[ColumnCount];
             for (int i = 0; i < columnIndices.Length; i++)
@@ -366,75 +437,6 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             MapIndexedInplace((i, j, x) => columns[j] ? Zero : x, Zeros.AllowSkip);
         }
 
-        /// <summary>
-        /// Indicates whether the current object is equal to another object of the same type.
-        /// </summary>
-        /// <param name="other">
-        /// An object to compare with this object.
-        /// </param>
-        /// <returns>
-        /// <c>true</c> if the current object is equal to the <paramref name="other"/> parameter; otherwise, <c>false</c>.
-        /// </returns>
-        public override bool Equals(MatrixStorage<T> other)
-        {
-            // Reject equality when the argument is null or has a different shape.
-            if (other == null || ColumnCount != other.ColumnCount || RowCount != other.RowCount)
-            {
-                return false;
-            }
-
-            // Accept if the argument is the same object as this.
-            if (ReferenceEquals(this, other))
-            {
-                return true;
-            }
-
-            var sparse = other as SparseCompressedRowMatrixStorage<T>;
-            if (sparse == null)
-            {
-                return base.Equals(other);
-            }
-
-            if (ValueCount != sparse.ValueCount)
-            {
-                // TODO: this is only correct if normalized
-                return false;
-            }
-
-            // If all else fails, perform element wise comparison.
-            for (var index = 0; index < ValueCount; index++)
-            {
-                // TODO: AlmostEquals
-                if (!Values[index].Equals(sparse.Values[index]) || ColumnIndices[index] != sparse.ColumnIndices[index])
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
-
-        /// <summary>
-        /// Returns a hash code for this instance.
-        /// </summary>
-        /// <returns>
-        /// A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table.
-        /// </returns>
-        public override int GetHashCode()
-        {
-            var values = Values;
-            var hashNum = Math.Min(ValueCount, 25);
-            int hash = 17;
-            unchecked
-            {
-                for (var i = 0; i < hashNum; i++)
-                {
-                    hash = hash*31 + values[i].GetHashCode();
-                }
-            }
-            return hash;
-        }
-
         // INITIALIZATION
 
         public static SparseCompressedRowMatrixStorage<T> OfMatrix(MatrixStorage<T> matrix)
@@ -959,7 +961,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
 
             if (existingData == ExistingData.Clear)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             for (int i = sourceRowIndex, row = 0; i < sourceRowIndex + rowCount; i++, row++)
@@ -1029,7 +1031,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
 
             if (existingData == ExistingData.Clear)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             // NOTE: potential for more efficient implementation
@@ -1642,7 +1644,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             var processZeros = zeros == Zeros.Include || !Zero.Equals(f(0, 1, Zero));
             if (existingData == ExistingData.Clear && !processZeros)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             if (processZeros)
@@ -1699,7 +1701,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
             var processZeros = zeros == Zeros.Include || !Zero.Equals(f(0, 1, Zero));
             if (existingData == ExistingData.Clear && !processZeros)
             {
-                target.Clear(targetRowIndex, rowCount, targetColumnIndex, columnCount);
+                target.ClearUnchecked(targetRowIndex, rowCount, targetColumnIndex, columnCount);
             }
 
             var rowOffset = targetRowIndex - sourceRowIndex;

src/Numerics/LinearAlgebra/Storage/SparseVectorStorage.cs

@@ -472,7 +472,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
         {
             if (existingData == ExistingData.Clear)
             {
-                target.Clear(rowIndex, 1, 0, Length);
+                target.ClearUnchecked(rowIndex, 1, 0, Length);
             }
 
             if (ValueCount == 0)
@@ -492,7 +492,7 @@ namespace MathNet.Numerics.LinearAlgebra.Storage
         {
             if (existingData == ExistingData.Clear)
             {
-                target.Clear(0, Length, columnIndex, 1);
+                target.ClearUnchecked(0, Length, columnIndex, 1);
             }
 
             if (ValueCount == 0)