From 0a0681f2a578188f2b0ce0b878a3923e7accfcb6 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Tue, 8 Mar 2022 02:27:38 +0530 Subject: [PATCH 01/15] Add : Size checks for adaboost and matix completion Signed-off-by: eshaanagarwal --- src/mlpack/methods/adaboost/adaboost_impl.hpp | 9 ++++++++- .../methods/matrix_completion/matrix_completion.cpp | 9 ++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/mlpack/methods/adaboost/adaboost_impl.hpp b/src/mlpack/methods/adaboost/adaboost_impl.hpp index 7d88c807af..af767d161f 100644 --- a/src/mlpack/methods/adaboost/adaboost_impl.hpp +++ b/src/mlpack/methods/adaboost/adaboost_impl.hpp @@ -27,6 +27,7 @@ #define MLPACK_METHODS_ADABOOST_ADABOOST_IMPL_HPP #include "adaboost.hpp" +#include namespace mlpack { namespace adaboost { @@ -71,6 +72,9 @@ double AdaBoost::Train( const size_t iterations, const double tolerance) { + // Sanity check on data + util::CheckSameDimensionality(data, labels, "Adaboost::Train()"); + // Clear information from previous runs. wl.clear(); alpha.clear(); @@ -242,7 +246,10 @@ void AdaBoost::Classify( const MatType& test, arma::Row& predictedLabels, arma::mat& probabilities) -{ +{ + // Sanity Check on Data + util::CheckSameDimensionality(test, predictedLabels, "Adaboost::Classify()"); + arma::Row tempPredictedLabels(test.n_cols); probabilities.zeros(numClasses, test.n_cols); diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index 6210b69453..6a2b9db523 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -11,6 +11,7 @@ */ #include "matrix_completion.hpp" +#include namespace mlpack { namespace matrix_completion { @@ -59,13 +60,7 @@ void MatrixCompletion::CheckValues() << "indices does not have 2 rows!" << std::endl; } - if (indices.n_cols != values.n_elem) - { - Log::Fatal << "MatrixCompletion::CheckValues(): the number of constraint " - << "indices (columns of constraint indices matrix) does not match the " - << "number of constraint values (length of constraint value vector)!" - << std::endl; - } + util::CheckSameSizes(values, indices, "MatrixCompletion::CheckValues()", "indices"); for (size_t i = 0; i < values.n_elem; ++i) { From 8714c77a959e708a6100d6c1a4813c086185bcff Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Wed, 9 Mar 2022 02:33:08 +0530 Subject: [PATCH 02/15] Add: size checks for kmeans and linear regression --- src/mlpack/methods/kmeans/kmeans_impl.hpp | 17 +++++------------ .../linear_regression/linear_regression.cpp | 14 +++++++++++--- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/mlpack/methods/kmeans/kmeans_impl.hpp b/src/mlpack/methods/kmeans/kmeans_impl.hpp index 4195da81ac..7e52464ced 100644 --- a/src/mlpack/methods/kmeans/kmeans_impl.hpp +++ b/src/mlpack/methods/kmeans/kmeans_impl.hpp @@ -14,6 +14,8 @@ #include #include +#include + namespace mlpack { namespace kmeans { @@ -161,15 +163,9 @@ Cluster(const MatType& data, // Check validity of initial guess. if (initialGuess) { - if (centroids.n_cols != clusters) - Log::Fatal << "KMeans::Cluster(): wrong number of initial cluster " - << "centroids (" << centroids.n_cols << ", should be " << clusters - << ")!" << std::endl; + util::CheckSameSizes(centroids, clusters, "KMeans::Cluster()", "clusters"); - if (centroids.n_rows != data.n_rows) - Log::Fatal << "KMeans::Cluster(): initial cluster centroids have wrong " - << " dimensionality (" << centroids.n_rows << ", should be " - << data.n_rows << ")!" << std::endl; + util::CheckSameDimensionality(data, centroids, "KMeans::Cluster()"); } // Use the partitioner to come up with the partition assignments and calculate @@ -288,10 +284,7 @@ Cluster(const MatType& data, // Now, the initial assignments. First determine if they are necessary. if (initialAssignmentGuess) { - if (assignments.n_elem != data.n_cols) - Log::Fatal << "KMeans::Cluster(): initial cluster assignments (length " - << assignments.n_elem << ") not the same size as the dataset (size " - << data.n_cols << ")!" << std::endl; + util::CheckSameSizes(data, assignments, "KMeans::Cluster()","assignments"); // Calculate initial centroids. arma::Row counts; diff --git a/src/mlpack/methods/linear_regression/linear_regression.cpp b/src/mlpack/methods/linear_regression/linear_regression.cpp index d3d4a2cd3f..744e184f18 100644 --- a/src/mlpack/methods/linear_regression/linear_regression.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression.cpp @@ -12,6 +12,7 @@ */ #include "linear_regression.hpp" #include +#include using namespace mlpack; using namespace mlpack::regression; @@ -57,6 +58,10 @@ double LinearRegression::Train(const arma::mat& predictors, // We store the number of rows and columns of the predictors. // Reminder: Armadillo stores the data transposed from how we think of it, // that is, columns are actually rows (see: column major order). + + // Sanity check on data + util::CheckSameSizes(predictors, responses, "LinearRegression::Train()"); + const size_t nCols = predictors.n_cols; arma::mat p = predictors; @@ -95,7 +100,7 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in the // dataset. - Log::Assert(points.n_rows == parameters.n_rows - 1); + util::CheckSameDimensionality(points, (size_t) (parameters.n_rows-1), "LinearRegression::Predict()", "points"); // Get the predictions, but this ignores the intercept value // (parameters[0]). predictions = arma::trans(parameters.subvec(1, parameters.n_elem - 1)) @@ -107,14 +112,17 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in // the dataset. - Log::Assert(points.n_rows == parameters.n_rows); + util::CheckSameDimensionality(points, parameters, "LinearRegression::Predict()", "points"); predictions = arma::trans(parameters) * points; } } double LinearRegression::ComputeError(const arma::mat& predictors, const arma::rowvec& responses) const -{ +{ + // Sanity check on data + util::CheckSameSizes(predictors, responses, "LinearRegression::Train()"); + // Get the number of columns and rows of the dataset. const size_t nCols = predictors.n_cols; const size_t nRows = predictors.n_rows; From 54f65060158fd883bb43fcb72d7062ce60c26cd3 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Thu, 10 Mar 2022 12:24:15 +0530 Subject: [PATCH 03/15] fix styling issue Signed-off-by: eshaanagarwal --- src/mlpack/methods/kmeans/kmeans_impl.hpp | 3 +-- src/mlpack/methods/linear_regression/linear_regression.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/mlpack/methods/kmeans/kmeans_impl.hpp b/src/mlpack/methods/kmeans/kmeans_impl.hpp index 7e52464ced..ac4f4cc748 100644 --- a/src/mlpack/methods/kmeans/kmeans_impl.hpp +++ b/src/mlpack/methods/kmeans/kmeans_impl.hpp @@ -164,7 +164,6 @@ Cluster(const MatType& data, if (initialGuess) { util::CheckSameSizes(centroids, clusters, "KMeans::Cluster()", "clusters"); - util::CheckSameDimensionality(data, centroids, "KMeans::Cluster()"); } @@ -284,7 +283,7 @@ Cluster(const MatType& data, // Now, the initial assignments. First determine if they are necessary. if (initialAssignmentGuess) { - util::CheckSameSizes(data, assignments, "KMeans::Cluster()","assignments"); + util::CheckSameSizes(data, assignments, "KMeans::Cluster()", "assignments"); // Calculate initial centroids. arma::Row counts; diff --git a/src/mlpack/methods/linear_regression/linear_regression.cpp b/src/mlpack/methods/linear_regression/linear_regression.cpp index 744e184f18..17f9e72a76 100644 --- a/src/mlpack/methods/linear_regression/linear_regression.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression.cpp @@ -100,7 +100,7 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in the // dataset. - util::CheckSameDimensionality(points, (size_t) (parameters.n_rows-1), "LinearRegression::Predict()", "points"); + util::CheckSameDimensionality(points, (size_t) (parameters.n_rows - 1), "LinearRegression::Predict()", "points"); // Get the predictions, but this ignores the intercept value // (parameters[0]). predictions = arma::trans(parameters.subvec(1, parameters.n_elem - 1)) From 05d74baffd841e09a607bf3b03e721d442d29e59 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Thu, 10 Mar 2022 16:30:49 +0530 Subject: [PATCH 04/15] fix size checks --- src/mlpack/methods/adaboost/adaboost_impl.hpp | 4 ++-- src/mlpack/methods/matrix_completion/matrix_completion.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mlpack/methods/adaboost/adaboost_impl.hpp b/src/mlpack/methods/adaboost/adaboost_impl.hpp index af767d161f..df67a9a29f 100644 --- a/src/mlpack/methods/adaboost/adaboost_impl.hpp +++ b/src/mlpack/methods/adaboost/adaboost_impl.hpp @@ -73,7 +73,7 @@ double AdaBoost::Train( const double tolerance) { // Sanity check on data - util::CheckSameDimensionality(data, labels, "Adaboost::Train()"); + util::CheckSameSizes(data, labels, "Adaboost::Train()"); // Clear information from previous runs. wl.clear(); @@ -248,7 +248,7 @@ void AdaBoost::Classify( arma::mat& probabilities) { // Sanity Check on Data - util::CheckSameDimensionality(test, predictedLabels, "Adaboost::Classify()"); + util::CheckSameSizes(test, predictedLabels, "Adaboost::Classify()"); arma::Row tempPredictedLabels(test.n_cols); diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index 6a2b9db523..ae44ccfad9 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -60,7 +60,7 @@ void MatrixCompletion::CheckValues() << "indices does not have 2 rows!" << std::endl; } - util::CheckSameSizes(values, indices, "MatrixCompletion::CheckValues()", "indices"); + util::CheckSameSizes(indices, values, "MatrixCompletion::CheckValues()", "indices"); for (size_t i = 0; i < values.n_elem; ++i) { From f4db66192bf2d9210265f02fcce3b0b0b27653ac Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Fri, 11 Mar 2022 12:19:37 +0530 Subject: [PATCH 05/15] remove incorrect checks in adaboost --- src/mlpack/methods/adaboost/adaboost_impl.hpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/mlpack/methods/adaboost/adaboost_impl.hpp b/src/mlpack/methods/adaboost/adaboost_impl.hpp index df67a9a29f..0cdc3da7ee 100644 --- a/src/mlpack/methods/adaboost/adaboost_impl.hpp +++ b/src/mlpack/methods/adaboost/adaboost_impl.hpp @@ -72,8 +72,6 @@ double AdaBoost::Train( const size_t iterations, const double tolerance) { - // Sanity check on data - util::CheckSameSizes(data, labels, "Adaboost::Train()"); // Clear information from previous runs. wl.clear(); @@ -247,8 +245,6 @@ void AdaBoost::Classify( arma::Row& predictedLabels, arma::mat& probabilities) { - // Sanity Check on Data - util::CheckSameSizes(test, predictedLabels, "Adaboost::Classify()"); arma::Row tempPredictedLabels(test.n_cols); From 2685d6909743c650137b1e31558c5b818e0d6b01 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Fri, 11 Mar 2022 19:30:22 +0530 Subject: [PATCH 06/15] fix matrix completion size-checks --- src/mlpack/core/util/size_checks.hpp | 3 +++ src/mlpack/methods/matrix_completion/matrix_completion.cpp | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mlpack/core/util/size_checks.hpp b/src/mlpack/core/util/size_checks.hpp index c2d60459b8..7025eb5508 100644 --- a/src/mlpack/core/util/size_checks.hpp +++ b/src/mlpack/core/util/size_checks.hpp @@ -33,6 +33,9 @@ inline void CheckSameSizes(const DataType& data, const std::string& callerDescription, const std::string& addInfo = "labels") { + Log::Assert( label.is_rowvec() == true, "Label matrix should be a row vector \ + ."); + if (data.n_cols != label.n_cols) { std::ostringstream oss; diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index ae44ccfad9..b0f7ed7dee 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -60,7 +60,7 @@ void MatrixCompletion::CheckValues() << "indices does not have 2 rows!" << std::endl; } - util::CheckSameSizes(indices, values, "MatrixCompletion::CheckValues()", "indices"); + util::CheckSameSizes(indices, (size_t)values.n_rows, "MatrixCompletion::CheckValues()", "values"); for (size_t i = 0; i < values.n_elem; ++i) { From 6883c30cb263f3226f664c58cebe70f124e64e97 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Fri, 11 Mar 2022 19:50:54 +0530 Subject: [PATCH 07/15] fixed redundancy in size-checks --- src/mlpack/core/util/size_checks.hpp | 2 +- src/mlpack/methods/matrix_completion/matrix_completion.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mlpack/core/util/size_checks.hpp b/src/mlpack/core/util/size_checks.hpp index 7025eb5508..ee22aba256 100644 --- a/src/mlpack/core/util/size_checks.hpp +++ b/src/mlpack/core/util/size_checks.hpp @@ -33,7 +33,7 @@ inline void CheckSameSizes(const DataType& data, const std::string& callerDescription, const std::string& addInfo = "labels") { - Log::Assert( label.is_rowvec() == true, "Label matrix should be a row vector \ + Log::Assert( label.is_rowvec(), "Label matrix should be a row vector \ ."); if (data.n_cols != label.n_cols) diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index b0f7ed7dee..a718c75b14 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -60,7 +60,8 @@ void MatrixCompletion::CheckValues() << "indices does not have 2 rows!" << std::endl; } - util::CheckSameSizes(indices, (size_t)values.n_rows, "MatrixCompletion::CheckValues()", "values"); + arma::mat transposeValues = values.t(); + util::CheckSameSizes(indices, transposeValues, "MatrixCompletion::CheckValues()", "values"); for (size_t i = 0; i < values.n_elem; ++i) { From ca353778a444bc826daed20c10438d4fe4277c89 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Fri, 11 Mar 2022 20:15:07 +0530 Subject: [PATCH 08/15] fix failed build --- src/mlpack/core/util/size_checks.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mlpack/core/util/size_checks.hpp b/src/mlpack/core/util/size_checks.hpp index ee22aba256..4a8dfe58d6 100644 --- a/src/mlpack/core/util/size_checks.hpp +++ b/src/mlpack/core/util/size_checks.hpp @@ -13,6 +13,7 @@ #ifndef MLPACK_UTIL_SIZE_CHECKS_HPP #define MLPACK_UTIL_SIZE_CHECKS_HPP +#include "log.hpp" namespace mlpack { namespace util { @@ -33,8 +34,7 @@ inline void CheckSameSizes(const DataType& data, const std::string& callerDescription, const std::string& addInfo = "labels") { - Log::Assert( label.is_rowvec(), "Label matrix should be a row vector \ - ."); + Log::Assert( label.is_rowvec(), "Label matrix should be a row vector."); if (data.n_cols != label.n_cols) { From 68e40a6b23b4fcea525f4b7692c28195ac1aa67d Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Fri, 11 Mar 2022 22:50:03 +0530 Subject: [PATCH 09/15] fix build issue by removing row vector assert condition --- src/mlpack/core/util/size_checks.hpp | 3 --- src/mlpack/methods/matrix_completion/matrix_completion.cpp | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/mlpack/core/util/size_checks.hpp b/src/mlpack/core/util/size_checks.hpp index 4a8dfe58d6..c2d60459b8 100644 --- a/src/mlpack/core/util/size_checks.hpp +++ b/src/mlpack/core/util/size_checks.hpp @@ -13,7 +13,6 @@ #ifndef MLPACK_UTIL_SIZE_CHECKS_HPP #define MLPACK_UTIL_SIZE_CHECKS_HPP -#include "log.hpp" namespace mlpack { namespace util { @@ -34,8 +33,6 @@ inline void CheckSameSizes(const DataType& data, const std::string& callerDescription, const std::string& addInfo = "labels") { - Log::Assert( label.is_rowvec(), "Label matrix should be a row vector."); - if (data.n_cols != label.n_cols) { std::ostringstream oss; diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index a718c75b14..d65e2f8217 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -53,7 +53,7 @@ MatrixCompletion::MatrixCompletion(const size_t m, } void MatrixCompletion::CheckValues() -{ +{ if (indices.n_rows != 2) { Log::Fatal << "MatrixCompletion::CheckValues(): matrix of constraint " From 5683066a660c948ef7b5363467191693ea3739ce Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Mon, 14 Mar 2022 22:25:27 +0530 Subject: [PATCH 10/15] fixed styling issues --- src/mlpack/methods/adaboost/adaboost_impl.hpp | 5 +---- src/mlpack/methods/kmeans/kmeans_impl.hpp | 1 - src/mlpack/methods/matrix_completion/matrix_completion.cpp | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/mlpack/methods/adaboost/adaboost_impl.hpp b/src/mlpack/methods/adaboost/adaboost_impl.hpp index 0cdc3da7ee..7d88c807af 100644 --- a/src/mlpack/methods/adaboost/adaboost_impl.hpp +++ b/src/mlpack/methods/adaboost/adaboost_impl.hpp @@ -27,7 +27,6 @@ #define MLPACK_METHODS_ADABOOST_ADABOOST_IMPL_HPP #include "adaboost.hpp" -#include namespace mlpack { namespace adaboost { @@ -72,7 +71,6 @@ double AdaBoost::Train( const size_t iterations, const double tolerance) { - // Clear information from previous runs. wl.clear(); alpha.clear(); @@ -244,8 +242,7 @@ void AdaBoost::Classify( const MatType& test, arma::Row& predictedLabels, arma::mat& probabilities) -{ - +{ arma::Row tempPredictedLabels(test.n_cols); probabilities.zeros(numClasses, test.n_cols); diff --git a/src/mlpack/methods/kmeans/kmeans_impl.hpp b/src/mlpack/methods/kmeans/kmeans_impl.hpp index ac4f4cc748..e8f782e916 100644 --- a/src/mlpack/methods/kmeans/kmeans_impl.hpp +++ b/src/mlpack/methods/kmeans/kmeans_impl.hpp @@ -16,7 +16,6 @@ #include #include - namespace mlpack { namespace kmeans { diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index d65e2f8217..a718c75b14 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -53,7 +53,7 @@ MatrixCompletion::MatrixCompletion(const size_t m, } void MatrixCompletion::CheckValues() -{ +{ if (indices.n_rows != 2) { Log::Fatal << "MatrixCompletion::CheckValues(): matrix of constraint " From b3b5c6827a685262ea12897564668dd356042c46 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Thu, 31 Mar 2022 04:22:53 +0530 Subject: [PATCH 11/15] fixed issues in styling --- src/mlpack/methods/linear_regression/linear_regression.cpp | 6 ++++-- src/mlpack/methods/matrix_completion/matrix_completion.cpp | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mlpack/methods/linear_regression/linear_regression.cpp b/src/mlpack/methods/linear_regression/linear_regression.cpp index 17f9e72a76..b36eeaf9d6 100644 --- a/src/mlpack/methods/linear_regression/linear_regression.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression.cpp @@ -100,7 +100,8 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in the // dataset. - util::CheckSameDimensionality(points, (size_t) (parameters.n_rows - 1), "LinearRegression::Predict()", "points"); + util::CheckSameDimensionality(points, (size_t) (parameters.n_rows - 1), + "LinearRegression::Predict()", "points"); // Get the predictions, but this ignores the intercept value // (parameters[0]). predictions = arma::trans(parameters.subvec(1, parameters.n_elem - 1)) @@ -112,7 +113,8 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in // the dataset. - util::CheckSameDimensionality(points, parameters, "LinearRegression::Predict()", "points"); + util::CheckSameDimensionality(points, parameters, + "LinearRegression::Predict()", "points"); predictions = arma::trans(parameters) * points; } } diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index a718c75b14..a857b6ee5e 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -61,7 +61,8 @@ void MatrixCompletion::CheckValues() } arma::mat transposeValues = values.t(); - util::CheckSameSizes(indices, transposeValues, "MatrixCompletion::CheckValues()", "values"); + util::CheckSameSizes(indices, transposeValues, + "MatrixCompletion::CheckValues()", "values"); for (size_t i = 0; i < values.n_elem; ++i) { From 0e9f05adbc9ec75c5604050b6012ba690f4d9ba8 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Tue, 5 Apr 2022 00:51:27 +0530 Subject: [PATCH 12/15] Add transpose parameter in size check --- src/mlpack/core/util/size_checks.hpp | 43 +++++++++++++++---- .../linear_regression/linear_regression.cpp | 6 +-- .../matrix_completion/matrix_completion.cpp | 5 +-- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/mlpack/core/util/size_checks.hpp b/src/mlpack/core/util/size_checks.hpp index c2d60459b8..7244f7217e 100644 --- a/src/mlpack/core/util/size_checks.hpp +++ b/src/mlpack/core/util/size_checks.hpp @@ -31,16 +31,43 @@ template inline void CheckSameSizes(const DataType& data, const LabelsType& label, const std::string& callerDescription, - const std::string& addInfo = "labels") -{ - if (data.n_cols != label.n_cols) + const std::string& addInfo = "labels", + const std::string& isTranspose = "none") +{ + if (isTranspose == "data") { - std::ostringstream oss; - oss << callerDescription << ": number of points (" << data.n_cols << ") " - << "does not match number of " << addInfo << " (" << label.n_cols - << ")!" << std::endl; - throw std::invalid_argument(oss.str()); + if (data.n_rows != label.n_cols) + { + std::ostringstream oss; + oss << callerDescription << ": number of points (" << data.n_cols << ") " + << "does not match number of " << addInfo << " (" << label.n_cols + << ")!" << std::endl; + throw std::invalid_argument(oss.str()); + } } + else if (isTranspose == "label") + { + if (data.n_cols != label.n_rows) + { + std::ostringstream oss; + oss << callerDescription << ": number of points (" << data.n_cols << ") " + << "does not match number of " << addInfo << " (" << label.n_cols + << ")!" << std::endl; + throw std::invalid_argument(oss.str()); + } + } + else + { + if (data.n_cols != label.n_cols) + { + std::ostringstream oss; + oss << callerDescription << ": number of points (" << data.n_cols << ") " + << "does not match number of " << addInfo << " (" << label.n_cols + << ")!" << std::endl; + throw std::invalid_argument(oss.str()); + } + } + } /** diff --git a/src/mlpack/methods/linear_regression/linear_regression.cpp b/src/mlpack/methods/linear_regression/linear_regression.cpp index b36eeaf9d6..fca974d41a 100644 --- a/src/mlpack/methods/linear_regression/linear_regression.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression.cpp @@ -59,7 +59,7 @@ double LinearRegression::Train(const arma::mat& predictors, // Reminder: Armadillo stores the data transposed from how we think of it, // that is, columns are actually rows (see: column major order). - // Sanity check on data + // Sanity check on data. util::CheckSameSizes(predictors, responses, "LinearRegression::Train()"); const size_t nCols = predictors.n_cols; @@ -121,8 +121,8 @@ void LinearRegression::Predict(const arma::mat& points, double LinearRegression::ComputeError(const arma::mat& predictors, const arma::rowvec& responses) const -{ - // Sanity check on data +{ + // Sanity check on data. util::CheckSameSizes(predictors, responses, "LinearRegression::Train()"); // Get the number of columns and rows of the dataset. diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index a857b6ee5e..5e274b62cf 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -60,9 +60,8 @@ void MatrixCompletion::CheckValues() << "indices does not have 2 rows!" << std::endl; } - arma::mat transposeValues = values.t(); - util::CheckSameSizes(indices, transposeValues, - "MatrixCompletion::CheckValues()", "values"); + util::CheckSameSizes(indices, values, + "MatrixCompletion::CheckValues()", "labels", "label"); for (size_t i = 0; i < values.n_elem; ++i) { From 845a5a0aed34910f8d7ef147ca8a726218963a16 Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Tue, 5 Apr 2022 13:02:00 +0530 Subject: [PATCH 13/15] add parameter documentation in size checks --- src/mlpack/core/util/size_checks.hpp | 50 ++++++------------- .../linear_regression/linear_regression.cpp | 5 ++ .../matrix_completion/matrix_completion.cpp | 2 +- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/src/mlpack/core/util/size_checks.hpp b/src/mlpack/core/util/size_checks.hpp index 7244f7217e..f5125a591b 100644 --- a/src/mlpack/core/util/size_checks.hpp +++ b/src/mlpack/core/util/size_checks.hpp @@ -26,48 +26,30 @@ namespace util { * error generation. * @param addInfo Name to use for labels for precise error generation. Default * is "labels"; for example, "weights" could also be used. + * @param isDataTranspose Bool parameter which can be set true to transpose data + * before size-check. Default is false. + * @param isLabelTranspose Bool parameter which can be set true to transpose label + * before size-check. Default is false. */ template inline void CheckSameSizes(const DataType& data, const LabelsType& label, const std::string& callerDescription, const std::string& addInfo = "labels", - const std::string& isTranspose = "none") + const bool& isDataTranspose = false, + const bool& isLabelTranspose = false) { - if (isTranspose == "data") - { - if (data.n_rows != label.n_cols) - { - std::ostringstream oss; - oss << callerDescription << ": number of points (" << data.n_cols << ") " - << "does not match number of " << addInfo << " (" << label.n_cols - << ")!" << std::endl; - throw std::invalid_argument(oss.str()); - } - } - else if (isTranspose == "label") - { - if (data.n_cols != label.n_rows) - { - std::ostringstream oss; - oss << callerDescription << ": number of points (" << data.n_cols << ") " - << "does not match number of " << addInfo << " (" << label.n_cols - << ")!" << std::endl; - throw std::invalid_argument(oss.str()); - } - } - else - { - if (data.n_cols != label.n_cols) - { - std::ostringstream oss; - oss << callerDescription << ": number of points (" << data.n_cols << ") " - << "does not match number of " << addInfo << " (" << label.n_cols - << ")!" << std::endl; - throw std::invalid_argument(oss.str()); - } - } + const size_t dataPoints = (isDataTranspose == true) ? data.n_rows : data.n_cols; + const size_t labelPoints = (isLabelTranspose == true) ? label.n_rows : label.n_cols; + if (dataPoints != labelPoints) + { + std::ostringstream oss; + oss << callerDescription << ": number of points (" << dataPoints << ") " + << "does not match number of " << addInfo << " (" << labelPoints + << ")!" << std::endl; + throw std::invalid_argument(oss.str()); + } } /** diff --git a/src/mlpack/methods/linear_regression/linear_regression.cpp b/src/mlpack/methods/linear_regression/linear_regression.cpp index fca974d41a..b054cbc313 100644 --- a/src/mlpack/methods/linear_regression/linear_regression.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression.cpp @@ -100,6 +100,11 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in the // dataset. + // Warning : If parameters.n_rows is 0, then size_t cast would lead + // it to capture UINT32_MAX. + // You might get error message like this - "mismatch size x and UNIT32_MAX" + // Checks to ensures parameters isn't an empty matrix will be added + // after PR #3140 gets merged util::CheckSameDimensionality(points, (size_t) (parameters.n_rows - 1), "LinearRegression::Predict()", "points"); // Get the predictions, but this ignores the intercept value diff --git a/src/mlpack/methods/matrix_completion/matrix_completion.cpp b/src/mlpack/methods/matrix_completion/matrix_completion.cpp index 5e274b62cf..0101760525 100644 --- a/src/mlpack/methods/matrix_completion/matrix_completion.cpp +++ b/src/mlpack/methods/matrix_completion/matrix_completion.cpp @@ -61,7 +61,7 @@ void MatrixCompletion::CheckValues() } util::CheckSameSizes(indices, values, - "MatrixCompletion::CheckValues()", "labels", "label"); + "MatrixCompletion::CheckValues()", "labels", false, true); for (size_t i = 0; i < values.n_elem; ++i) { From cb35eba204c328cda8864aa742f2d3ea6c6b294d Mon Sep 17 00:00:00 2001 From: eshaanagarwal Date: Wed, 13 Apr 2022 18:22:03 +0530 Subject: [PATCH 14/15] fix error in size_t cast Signed-off-by: eshaanagarwal --- .../methods/linear_regression/linear_regression.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/mlpack/methods/linear_regression/linear_regression.cpp b/src/mlpack/methods/linear_regression/linear_regression.cpp index b054cbc313..d046c129a3 100644 --- a/src/mlpack/methods/linear_regression/linear_regression.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression.cpp @@ -100,13 +100,9 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in the // dataset. - // Warning : If parameters.n_rows is 0, then size_t cast would lead - // it to capture UINT32_MAX. - // You might get error message like this - "mismatch size x and UNIT32_MAX" - // Checks to ensures parameters isn't an empty matrix will be added - // after PR #3140 gets merged - util::CheckSameDimensionality(points, (size_t) (parameters.n_rows - 1), - "LinearRegression::Predict()", "points"); + const size_t labels = (parameters.n_rows == 0) ? size_t(0) : size_t(parameters.n_rows - 1); + util::CheckSameDimensionality(points, labels, "LinearRegression::Predict()", + "points"); // Get the predictions, but this ignores the intercept value // (parameters[0]). predictions = arma::trans(parameters.subvec(1, parameters.n_elem - 1)) From 01fa1241cee30b9e2cd47d61137dc733994a74af Mon Sep 17 00:00:00 2001 From: Eshaan Agarwal Date: Wed, 13 Apr 2022 20:13:06 +0530 Subject: [PATCH 15/15] Fix style issues Co-authored-by: Ryan Curtin --- src/mlpack/methods/linear_regression/linear_regression.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mlpack/methods/linear_regression/linear_regression.cpp b/src/mlpack/methods/linear_regression/linear_regression.cpp index d046c129a3..774425ebc7 100644 --- a/src/mlpack/methods/linear_regression/linear_regression.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression.cpp @@ -100,7 +100,9 @@ void LinearRegression::Predict(const arma::mat& points, { // We want to be sure we have the correct number of dimensions in the // dataset. - const size_t labels = (parameters.n_rows == 0) ? size_t(0) : size_t(parameters.n_rows - 1); + // Prevent underflow. + const size_t labels = (parameters.n_rows == 0) ? size_t(0) : + size_t(parameters.n_rows - 1); util::CheckSameDimensionality(points, labels, "LinearRegression::Predict()", "points"); // Get the predictions, but this ignores the intercept value