From 94a37cb1c13a62dc8ca28919025fb89b426d8d12 Mon Sep 17 00:00:00 2001 From: Christoph Ruegg Date: Tue, 20 Jan 2015 23:00:26 +0100 Subject: [PATCH] LA: More consistent range checking in MatrixStorage.Clear and related routines #284 --- src/Numerics/LinearAlgebra/Matrix.cs | 48 +----- .../Storage/DenseColumnMajorMatrixStorage.cs | 10 +- .../Storage/DiagonalMatrixStorage.cs | 74 +++++---- .../LinearAlgebra/Storage/MatrixStorage.cs | 150 +++++++++++------ .../SparseCompressedRowMatrixStorage.cs | 154 +++++++++--------- .../Storage/SparseVectorStorage.cs | 4 +- 6 files changed, 231 insertions(+), 209 deletions(-) diff --git a/src/Numerics/LinearAlgebra/Matrix.cs b/src/Numerics/LinearAlgebra/Matrix.cs index f139ecce..4e936026 100644 --- a/src/Numerics/LinearAlgebra/Matrix.cs +++ b/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); } /// @@ -171,7 +171,7 @@ namespace MathNet.Numerics.LinearAlgebra throw new ArgumentOutOfRangeException("columnIndex"); } - Storage.Clear(0, RowCount, columnIndex, 1); + Storage.ClearUnchecked(0, RowCount, columnIndex, 1); } /// @@ -179,19 +179,6 @@ namespace MathNet.Numerics.LinearAlgebra /// 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 /// 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); } /// - /// Sets all values of a submatrix to zero. + /// Sets all values of a sub-matrix to zero. /// 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); } diff --git a/src/Numerics/LinearAlgebra/Storage/DenseColumnMajorMatrixStorage.cs b/src/Numerics/LinearAlgebra/Storage/DenseColumnMajorMatrixStorage.cs index ec4eff1e..e2a43032 100644 --- a/src/Numerics/LinearAlgebra/Storage/DenseColumnMajorMatrixStorage.cs +++ b/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++) { diff --git a/src/Numerics/LinearAlgebra/Storage/DiagonalMatrixStorage.cs b/src/Numerics/LinearAlgebra/Storage/DiagonalMatrixStorage.cs index 593be38f..0242a783 100644 --- a/src/Numerics/LinearAlgebra/Storage/DiagonalMatrixStorage.cs +++ b/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; - } - } - /// /// Indicates whether the current object is equal to another object of the same type. /// @@ -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 OfMatrix(MatrixStorage 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) diff --git a/src/Numerics/LinearAlgebra/Storage/MatrixStorage.cs b/src/Numerics/LinearAlgebra/Storage/MatrixStorage.cs index c45e08f9..19f3846e 100644 --- a/src/Numerics/LinearAlgebra/Storage/MatrixStorage.cs +++ b/src/Numerics/LinearAlgebra/Storage/MatrixStorage.cs @@ -128,52 +128,6 @@ namespace MathNet.Numerics.LinearAlgebra.Storage /// WARNING: This method is not thread safe. Use "lock" with it and be sure to avoid deadlocks. 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); - } - } - } - /// /// Indicates whether the current object is equal to another object of the same type. /// @@ -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 target, ExistingData existingData = ExistingData.Clear) diff --git a/src/Numerics/LinearAlgebra/Storage/SparseCompressedRowMatrixStorage.cs b/src/Numerics/LinearAlgebra/Storage/SparseCompressedRowMatrixStorage.cs index 6c4bb6c6..b87a217b 100644 --- a/src/Numerics/LinearAlgebra/Storage/SparseCompressedRowMatrixStorage.cs +++ b/src/Numerics/LinearAlgebra/Storage/SparseCompressedRowMatrixStorage.cs @@ -287,12 +287,83 @@ namespace MathNet.Numerics.LinearAlgebra.Storage MapInplace(x => x, Zeros.AllowSkip); } + /// + /// Indicates whether the current object is equal to another object of the same type. + /// + /// + /// An object to compare with this object. + /// + /// + /// true if the current object is equal to the parameter; otherwise, false. + /// + public override bool Equals(MatrixStorage 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; + 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; + } + + /// + /// Returns a hash code for this instance. + /// + /// + /// A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table. + /// + 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); } - /// - /// Indicates whether the current object is equal to another object of the same type. - /// - /// - /// An object to compare with this object. - /// - /// - /// true if the current object is equal to the parameter; otherwise, false. - /// - public override bool Equals(MatrixStorage 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; - 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; - } - - /// - /// Returns a hash code for this instance. - /// - /// - /// A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table. - /// - 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 OfMatrix(MatrixStorage 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; diff --git a/src/Numerics/LinearAlgebra/Storage/SparseVectorStorage.cs b/src/Numerics/LinearAlgebra/Storage/SparseVectorStorage.cs index 0f919562..8f90a1eb 100644 --- a/src/Numerics/LinearAlgebra/Storage/SparseVectorStorage.cs +++ b/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)