From 8e97af4ca0e08996a3fb4b3e23feddb2306cf7b3 Mon Sep 17 00:00:00 2001 From: jeffinsam Date: Tue, 19 Nov 2019 18:20:46 +0530 Subject: [PATCH 1/4] fix:don't assume that arma::eig_sym() always succeeds --- src/mlpack/core/math/lin_alg.cpp | 9 +++++++-- src/mlpack/core/math/lin_alg.hpp | 2 +- .../methods/gmm/positive_definite_constraint.hpp | 12 ++++++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/mlpack/core/math/lin_alg.cpp b/src/mlpack/core/math/lin_alg.cpp index 822625a59b..9612c30d42 100644 --- a/src/mlpack/core/math/lin_alg.cpp +++ b/src/mlpack/core/math/lin_alg.cpp @@ -77,7 +77,7 @@ void mlpack::math::WhitenUsingSVD(const arma::mat& x, * Whitens a matrix using the eigendecomposition of the covariance matrix. * Whitening means the covariance matrix of the result is the identity matrix. */ -void mlpack::math::WhitenUsingEig(const arma::mat& x, +bool mlpack::math::WhitenUsingEig(const arma::mat& x, arma::mat& xWhitened, arma::mat& whiteningMatrix) { @@ -85,7 +85,11 @@ void mlpack::math::WhitenUsingEig(const arma::mat& x, arma::vec eigenvalues; // Get eigenvectors of covariance of input matrix. - eig_sym(eigenvalues, eigenvectors, mlpack::math::ColumnCovariance(x)); + if (!eig_sym(eigenvalues, eigenvectors, mlpack::math::ColumnCovariance(x))) + { + Log::Fatal << "Failed Whitening" << std::endl; + return false; + } // Generate diagonal matrix using 1 / sqrt(eigenvalues) for each value. VectorPower(eigenvalues, -0.5); @@ -97,6 +101,7 @@ void mlpack::math::WhitenUsingEig(const arma::mat& x, // Now apply the whitening matrix. xWhitened = whiteningMatrix * x; + return true; } /** diff --git a/src/mlpack/core/math/lin_alg.hpp b/src/mlpack/core/math/lin_alg.hpp index 07b9bf720d..23addf2c86 100644 --- a/src/mlpack/core/math/lin_alg.hpp +++ b/src/mlpack/core/math/lin_alg.hpp @@ -49,7 +49,7 @@ void WhitenUsingSVD(const arma::mat& x, * Whitens a matrix using the eigendecomposition of the covariance matrix. * Whitening means the covariance matrix of the result is the identity matrix. */ -void WhitenUsingEig(const arma::mat& x, +bool WhitenUsingEig(const arma::mat& x, arma::mat& xWhitened, arma::mat& whiteningMatrix); diff --git a/src/mlpack/methods/gmm/positive_definite_constraint.hpp b/src/mlpack/methods/gmm/positive_definite_constraint.hpp index c41d38de46..3c0cc38877 100644 --- a/src/mlpack/methods/gmm/positive_definite_constraint.hpp +++ b/src/mlpack/methods/gmm/positive_definite_constraint.hpp @@ -32,8 +32,9 @@ class PositiveDefiniteConstraint * and ensure each value on the diagonal is at least 1e-50. * * @param covariance Covariance matrix. + * @return bool True on success */ - static void ApplyConstraint(arma::mat& covariance) + static bool ApplyConstraint(arma::mat& covariance) { // What we want to do is make sure that the matrix is positive definite and // that the condition number isn't too large. We also need to ensure that @@ -41,7 +42,13 @@ class PositiveDefiniteConstraint // eigenvalues are at least 1e-50). arma::vec eigval; arma::mat eigvec; - arma::eig_sym(eigval, eigvec, covariance); + covariance = arma::symmatu(covariance); + if (!arma::eig_sym(eigval, eigvec, covariance)) + { + Log::Fatal << "applying to constraint could not be accomplished " + << std::endl; + return false; + } // If the matrix is not positive definite or if the condition number is // large, we must project it back onto the cone of positive definite @@ -60,6 +67,7 @@ class PositiveDefiniteConstraint // Now reassemble the covariance matrix. covariance = eigvec * arma::diagmat(eigval) * eigvec.t(); } + return true; } /** From ac9b8701c8024e9fa3fe2803973015f4cd5eaac1 Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Wed, 29 Jan 2020 10:30:22 +0530 Subject: [PATCH 2/4] Removing unused function and add check for eig_sym and chol --- .../core/dists/gaussian_distribution.cpp | 5 ++- src/mlpack/core/math/lin_alg.cpp | 31 ------------------- src/mlpack/core/math/lin_alg.hpp | 7 ----- .../gmm/eigenvalue_ratio_constraint.hpp | 7 ++++- src/mlpack/methods/gmm/gmm.cpp | 7 ++++- .../gmm/positive_definite_constraint.hpp | 6 ++-- .../kernel_pca/kernel_rules/naive_method.hpp | 6 +++- .../kernel_rules/nystroem_method.hpp | 6 +++- src/mlpack/tests/lin_alg_test.cpp | 31 ------------------- 9 files changed, 28 insertions(+), 78 deletions(-) diff --git a/src/mlpack/core/dists/gaussian_distribution.cpp b/src/mlpack/core/dists/gaussian_distribution.cpp index b112a7c3c9..8c05de2da1 100644 --- a/src/mlpack/core/dists/gaussian_distribution.cpp +++ b/src/mlpack/core/dists/gaussian_distribution.cpp @@ -39,8 +39,11 @@ void GaussianDistribution::Covariance(arma::mat&& covariance) void GaussianDistribution::FactorCovariance() { // On Armadillo < 4.500, the "lower" option isn't available. - covLower = arma::chol(covariance, "lower"); + if (!arma::chol(covLower, covariance, "lower")) + { + Log::Warn << "Cholesky decomposition failed." << std::endl; + } // Comment from rcurtin: // // I think the use of the word "interpret" in the Armadillo documentation diff --git a/src/mlpack/core/math/lin_alg.cpp b/src/mlpack/core/math/lin_alg.cpp index 9612c30d42..48919ce56f 100644 --- a/src/mlpack/core/math/lin_alg.cpp +++ b/src/mlpack/core/math/lin_alg.cpp @@ -73,37 +73,6 @@ void mlpack::math::WhitenUsingSVD(const arma::mat& x, xWhitened = whiteningMatrix * x; } -/** - * Whitens a matrix using the eigendecomposition of the covariance matrix. - * Whitening means the covariance matrix of the result is the identity matrix. - */ -bool mlpack::math::WhitenUsingEig(const arma::mat& x, - arma::mat& xWhitened, - arma::mat& whiteningMatrix) -{ - arma::mat diag, eigenvectors; - arma::vec eigenvalues; - - // Get eigenvectors of covariance of input matrix. - if (!eig_sym(eigenvalues, eigenvectors, mlpack::math::ColumnCovariance(x))) - { - Log::Fatal << "Failed Whitening" << std::endl; - return false; - } - - // Generate diagonal matrix using 1 / sqrt(eigenvalues) for each value. - VectorPower(eigenvalues, -0.5); - diag.zeros(eigenvalues.n_elem, eigenvalues.n_elem); - diag.diag() = eigenvalues; - - // Our whitening matrix is diag(1 / sqrt(eigenvectors)) * eigenvalues. - whiteningMatrix = diag * trans(eigenvectors); - - // Now apply the whitening matrix. - xWhitened = whiteningMatrix * x; - return true; -} - /** * Overwrites a dimension-N vector to a random vector on the unit sphere in R^N. */ diff --git a/src/mlpack/core/math/lin_alg.hpp b/src/mlpack/core/math/lin_alg.hpp index 23addf2c86..c498920752 100644 --- a/src/mlpack/core/math/lin_alg.hpp +++ b/src/mlpack/core/math/lin_alg.hpp @@ -45,13 +45,6 @@ void WhitenUsingSVD(const arma::mat& x, arma::mat& xWhitened, arma::mat& whiteningMatrix); -/** - * Whitens a matrix using the eigendecomposition of the covariance matrix. - * Whitening means the covariance matrix of the result is the identity matrix. - */ -bool WhitenUsingEig(const arma::mat& x, - arma::mat& xWhitened, - arma::mat& whiteningMatrix); /** * Overwrites a dimension-N vector to a random vector on the unit sphere in R^N. diff --git a/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp b/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp index c33888a7d5..eb83bbf5ca 100644 --- a/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp +++ b/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp @@ -64,7 +64,12 @@ class EigenvalueRatioConstraint // Eigendecompose the matrix. arma::vec eigenvalues; arma::mat eigenvectors; - arma::eig_sym(eigenvalues, eigenvectors, covariance); + covariance = arma::symmatu(covariance); + if (!arma::eig_sym(eigenvalues, eigenvectors, covariance)) + { + Log::Warn << "applying to constraint could not be accomplished." + << std::endl; + } // Change the eigenvalues to what we are forcing them to be. There // shouldn't be any negative eigenvalues anyway, so it doesn't matter if we diff --git a/src/mlpack/methods/gmm/gmm.cpp b/src/mlpack/methods/gmm/gmm.cpp index 7eb7672fbc..680f6d320e 100644 --- a/src/mlpack/methods/gmm/gmm.cpp +++ b/src/mlpack/methods/gmm/gmm.cpp @@ -117,7 +117,12 @@ arma::vec GMM::Random() const } } - return trans(chol(dists[gaussian].Covariance())) * + arma::mat cholDecomp; + if (!arma::chol(cholDecomp, dists[gaussian].Covariance())) + { + Log::Warn << "Cholesky decomposition failed." << std::endl; + } + return trans(cholDecomp) * arma::randn(dimensionality) + dists[gaussian].Mean(); } diff --git a/src/mlpack/methods/gmm/positive_definite_constraint.hpp b/src/mlpack/methods/gmm/positive_definite_constraint.hpp index 3c0cc38877..84e3558701 100644 --- a/src/mlpack/methods/gmm/positive_definite_constraint.hpp +++ b/src/mlpack/methods/gmm/positive_definite_constraint.hpp @@ -34,7 +34,7 @@ class PositiveDefiniteConstraint * @param covariance Covariance matrix. * @return bool True on success */ - static bool ApplyConstraint(arma::mat& covariance) + static void ApplyConstraint(arma::mat& covariance) { // What we want to do is make sure that the matrix is positive definite and // that the condition number isn't too large. We also need to ensure that @@ -45,9 +45,8 @@ class PositiveDefiniteConstraint covariance = arma::symmatu(covariance); if (!arma::eig_sym(eigval, eigvec, covariance)) { - Log::Fatal << "applying to constraint could not be accomplished " + Log::Warn << "applying to constraint could not be accomplished." << std::endl; - return false; } // If the matrix is not positive definite or if the condition number is @@ -67,7 +66,6 @@ class PositiveDefiniteConstraint // Now reassemble the covariance matrix. covariance = eigvec * arma::diagmat(eigval) * eigvec.t(); } - return true; } /** diff --git a/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp b/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp index 567ab51b83..ee2705b74b 100644 --- a/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp +++ b/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp @@ -73,7 +73,11 @@ class NaiveKernelRule kernelMatrix += arma::sum(rowMean) / kernelMatrix.n_cols; // Eigendecompose the centered kernel matrix. - arma::eig_sym(eigval, eigvec, kernelMatrix); + kernelMatrix = arma::symmatu(kernelMatrix); + if (!arma::eig_sym(eigval, eigvec, kernelMatrix)) + { + Log::Warn << "Failed to construct the kernel matrix." << std::endl; + } // Swap the eigenvalues since they are ordered backwards (we need largest to // smallest). diff --git a/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp b/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp index 3ee0b0aed7..bf658dc0b7 100644 --- a/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp +++ b/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp @@ -64,7 +64,11 @@ class NystroemKernelRule G += arma::sum(colMean) / G.n_rows; // Eigendecompose the centered kernel matrix. - arma::eig_sym(eigval, eigvec, transformedData); + transformedData = arma::symmatu(transformedData); + if (!arma::eig_sym(eigval, eigvec, transformedData)) + { + Log::Warn << "Failed to construct the kernel matrix." << std::endl; + } // Swap the eigenvalues since they are ordered backwards (we need largest // to smallest). diff --git a/src/mlpack/tests/lin_alg_test.cpp b/src/mlpack/tests/lin_alg_test.cpp index cd1f3ced85..ecccae50e7 100644 --- a/src/mlpack/tests/lin_alg_test.cpp +++ b/src/mlpack/tests/lin_alg_test.cpp @@ -78,37 +78,6 @@ BOOST_AUTO_TEST_CASE(TestCenterB) BOOST_REQUIRE_CLOSE(tmp_out(row, col), (double) (col - 2.5) * row, 1e-5); } -BOOST_AUTO_TEST_CASE(TestWhitenUsingEig) -{ - // After whitening using eigendecomposition, the covariance of - // our matrix will be I (or something very close to that). - // We are loading a matrix from an external file... bad choice. - mat tmp, tmp_centered, whitened, whitening_matrix; - - data::Load("trainSet.csv", tmp); - Center(tmp, tmp_centered); - WhitenUsingEig(tmp_centered, whitened, whitening_matrix); - - mat newcov = mlpack::math::ColumnCovariance(whitened); - for (int row = 0; row < 5; row++) - { - for (int col = 0; col < 5; col++) - { - if (row == col) - { - // diagonal will be 0 in the case of any zero-valued eigenvalues - // (rank-deficient covariance case) - if (std::abs(newcov(row, col)) > 1e-10) - BOOST_REQUIRE_CLOSE(newcov(row, col), 1.0, 1e-10); - } - else - { - BOOST_REQUIRE_SMALL(newcov(row, col), 1e-10); - } - } - } -} - BOOST_AUTO_TEST_CASE(TestOrthogonalize) { // Generate a random matrix; then, orthogonalize it and test if it's From 034bf1c901556f6005e49bbcddc045e2706c70a3 Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Thu, 30 Jan 2020 11:07:29 +0530 Subject: [PATCH 3/4] Replace Log::warn with log::fatal --- src/mlpack/core/dists/gaussian_distribution.cpp | 2 +- src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp | 2 +- src/mlpack/methods/gmm/gmm.cpp | 2 +- src/mlpack/methods/gmm/positive_definite_constraint.hpp | 3 +-- src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp | 2 +- src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/mlpack/core/dists/gaussian_distribution.cpp b/src/mlpack/core/dists/gaussian_distribution.cpp index 8c05de2da1..812b9d6dd7 100644 --- a/src/mlpack/core/dists/gaussian_distribution.cpp +++ b/src/mlpack/core/dists/gaussian_distribution.cpp @@ -42,7 +42,7 @@ void GaussianDistribution::FactorCovariance() if (!arma::chol(covLower, covariance, "lower")) { - Log::Warn << "Cholesky decomposition failed." << std::endl; + Log::Fatal << "Cholesky decomposition failed." << std::endl; } // Comment from rcurtin: // diff --git a/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp b/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp index eb83bbf5ca..0aa9760e74 100644 --- a/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp +++ b/src/mlpack/methods/gmm/eigenvalue_ratio_constraint.hpp @@ -67,7 +67,7 @@ class EigenvalueRatioConstraint covariance = arma::symmatu(covariance); if (!arma::eig_sym(eigenvalues, eigenvectors, covariance)) { - Log::Warn << "applying to constraint could not be accomplished." + Log::Fatal << "applying to constraint could not be accomplished." << std::endl; } diff --git a/src/mlpack/methods/gmm/gmm.cpp b/src/mlpack/methods/gmm/gmm.cpp index 680f6d320e..aef6c9cda8 100644 --- a/src/mlpack/methods/gmm/gmm.cpp +++ b/src/mlpack/methods/gmm/gmm.cpp @@ -120,7 +120,7 @@ arma::vec GMM::Random() const arma::mat cholDecomp; if (!arma::chol(cholDecomp, dists[gaussian].Covariance())) { - Log::Warn << "Cholesky decomposition failed." << std::endl; + Log::Fatal << "Cholesky decomposition failed." << std::endl; } return trans(cholDecomp) * arma::randn(dimensionality) + dists[gaussian].Mean(); diff --git a/src/mlpack/methods/gmm/positive_definite_constraint.hpp b/src/mlpack/methods/gmm/positive_definite_constraint.hpp index 84e3558701..2e5d33892d 100644 --- a/src/mlpack/methods/gmm/positive_definite_constraint.hpp +++ b/src/mlpack/methods/gmm/positive_definite_constraint.hpp @@ -32,7 +32,6 @@ class PositiveDefiniteConstraint * and ensure each value on the diagonal is at least 1e-50. * * @param covariance Covariance matrix. - * @return bool True on success */ static void ApplyConstraint(arma::mat& covariance) { @@ -45,7 +44,7 @@ class PositiveDefiniteConstraint covariance = arma::symmatu(covariance); if (!arma::eig_sym(eigval, eigvec, covariance)) { - Log::Warn << "applying to constraint could not be accomplished." + Log::Fatal << "applying to constraint could not be accomplished." << std::endl; } diff --git a/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp b/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp index ee2705b74b..193a64a30c 100644 --- a/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp +++ b/src/mlpack/methods/kernel_pca/kernel_rules/naive_method.hpp @@ -76,7 +76,7 @@ class NaiveKernelRule kernelMatrix = arma::symmatu(kernelMatrix); if (!arma::eig_sym(eigval, eigvec, kernelMatrix)) { - Log::Warn << "Failed to construct the kernel matrix." << std::endl; + Log::Fatal << "Failed to construct the kernel matrix." << std::endl; } // Swap the eigenvalues since they are ordered backwards (we need largest to diff --git a/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp b/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp index bf658dc0b7..cdbc0a55fc 100644 --- a/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp +++ b/src/mlpack/methods/kernel_pca/kernel_rules/nystroem_method.hpp @@ -67,7 +67,7 @@ class NystroemKernelRule transformedData = arma::symmatu(transformedData); if (!arma::eig_sym(eigval, eigvec, transformedData)) { - Log::Warn << "Failed to construct the kernel matrix." << std::endl; + Log::Fatal << "Failed to construct the kernel matrix." << std::endl; } // Swap the eigenvalues since they are ordered backwards (we need largest From e87c9a97dc5362ec5ed477c1009f48066454ea86 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 7 Feb 2020 22:47:45 -0500 Subject: [PATCH 4/4] Update HISTORY. --- HISTORY.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index 03fdf60de4..7a3058d28f 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -24,9 +24,12 @@ * CMake fix for finding STB include directory (#2145). * Add normalization support for CF binding (#2136). - + * Add Mish activation function (#2158). + * Better error handling of eigendecompositions and Cholesky decompositions + (#2088, #1840). + ### mlpack 3.2.2 ###### 2019-11-26 * Add `valid` and `same` padding option in `Convolution` and `Atrous