Skip to content

Vectorize checks called by compiler #2556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
b7dd98c
Adds vectorized versions of the checks used by the compiler
SteveBronder Aug 5, 2021
46ce2e8
start adding vectorized tests
SteveBronder Aug 6, 2021
11d11b4
adds docs and finishes tests
SteveBronder Aug 6, 2021
df9255c
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 6, 2021
83224ea
fix cpplint
SteveBronder Aug 6, 2021
a5ffbd5
check dims for matrices and vectors for sizes
SteveBronder Aug 9, 2021
e82fd29
fix mem error in test for check_greater
SteveBronder Aug 9, 2021
a678bf4
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 9, 2021
f892c31
Fixup na vectorized check
SteveBronder Aug 9, 2021
e318a49
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 9, 2021
e3f196c
fix error msg
SteveBronder Aug 9, 2021
c0fd490
space after equal to
SteveBronder Aug 10, 2021
6cba218
Merge remote-tracking branch 'origin/develop' into fix/check-less-gre…
SteveBronder Aug 10, 2021
64c6019
update profile tests to sleep for a duration
SteveBronder Aug 10, 2021
f4ed5c1
refactor docs for err checks
SteveBronder Aug 13, 2021
87b15e5
update docs
SteveBronder Aug 13, 2021
383f6e9
update docs
SteveBronder Aug 13, 2021
3ff2da0
update for specializations for looking over standard vectors of scalars
SteveBronder Aug 17, 2021
2d1029c
Merge commit '02dc560b022f328f917af80a1b9b7f1feb249ee4' into HEAD
yashikno Aug 17, 2021
2b1b480
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 17, 2021
6bf2271
Add unlikely to check_greater/less
SteveBronder Aug 17, 2021
71e8b6b
Merge branch 'fix/check-less-greater' of github.com:stan-dev/math int…
SteveBronder Aug 17, 2021
82dc731
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 17, 2021
9994174
inline string construction for greater/less test
SteveBronder Aug 18, 2021
6b14ee7
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 18, 2021
07f31cb
value_of_rec for scalar case
SteveBronder Aug 18, 2021
ea2a9eb
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 18, 2021
3a36e2b
value_of_rec for high low scalar in error
SteveBronder Aug 18, 2021
f96a182
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 18, 2021
4273c1e
Merge remote-tracking branch 'origin/develop' into fix/check-less-gre…
SteveBronder Aug 19, 2021
f26b24c
Passes an idx parameter pack for lazy name construction
SteveBronder Aug 19, 2021
4a02109
Do lazy construction in check_less/greater
SteveBronder Aug 19, 2021
840075f
remove initializer list and pass in as arguments for check_greater_or…
SteveBronder Aug 19, 2021
aa4d183
pass error handling as args to lambda
SteveBronder Aug 19, 2021
64c804f
update docs
SteveBronder Aug 19, 2021
31b69b6
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stan/math/prim/err.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@
#include <stan/math/prim/err/out_of_range.hpp>
#include <stan/math/prim/err/system_error.hpp>
#include <stan/math/prim/err/throw_domain_error.hpp>
#include <stan/math/prim/err/throw_domain_error_mat.hpp>
#include <stan/math/prim/err/throw_domain_error_vec.hpp>
#include <stan/math/prim/err/validate_non_negative_index.hpp>
#include <stan/math/prim/err/validate_positive_index.hpp>
#include <stan/math/prim/err/validate_unit_vector_index.hpp>

#endif
58 changes: 44 additions & 14 deletions stan/math/prim/err/check_cholesky_factor.hpp
Original file line number Diff line number Diff line change
@@ -1,41 +1,71 @@
#ifndef STAN_MATH_PRIM_ERR_CHECK_CHOLESKY_FACTOR_HPP
#define STAN_MATH_PRIM_ERR_CHECK_CHOLESKY_FACTOR_HPP

#include <stan/math/prim/meta.hpp>
#include <stan/math/prim/fun/Eigen.hpp>
#include <stan/math/prim/meta.hpp>
#include <stan/math/prim/fun/to_ref.hpp>
#include <stan/math/prim/fun/value_of_rec.hpp>
#include <stan/math/prim/err/check_positive.hpp>
#include <stan/math/prim/err/check_less_or_equal.hpp>
#include <stan/math/prim/err/check_lower_triangular.hpp>
#include <stan/math/prim/err/make_iter_name.hpp>

namespace stan {
namespace math {

/**
* Check if the specified matrix is a valid Cholesky factor.
* A Cholesky factor is a lower triangular matrix whose diagonal
* elements are all positive. Note that Cholesky factors need not
* be square, but require at least as many rows M as columns N
* (i.e., M &gt;= N).
* @tparam EigMat Type of the Cholesky factor (must be derived from \c
* Eigen::MatrixBase)
* Throw an exception if the specified matrix is not a valid Cholesky factor. A
* Cholesky factor is a lower triangular matrix whose diagonal elements are all
* positive. Note that Cholesky factors need not be square, but require at
* least as many rows M as columns N (i.e., `M >= N`).
* @tparam Mat Type inheriting from `MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1 or a `var_value` with the var's
* inner type inheriting from `Eigen::MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1
* @param function Function name (for error messages)
* @param name Variable name (for error messages)
* @param y Matrix to test
* @throw <code>std::domain_error</code> if y is not a valid Cholesky
* factor, if number of rows is less than the number of columns,
* if there are 0 columns, or if any element in matrix is NaN
* @throw `std::domain_error` if y is not a valid Cholesky factor, if number of
* rows is less than the number of columns, if there are 0 columns, or if any
* element in matrix is `NaN`
*/
template <typename EigMat, require_eigen_t<EigMat>* = nullptr>
template <typename Mat, require_matrix_t<Mat>* = nullptr>
inline void check_cholesky_factor(const char* function, const char* name,
const EigMat& y) {
const Mat& y) {
check_less_or_equal(function, "columns and rows of Cholesky factor", y.cols(),
y.rows());
check_positive(function, "columns of Cholesky factor", y.cols());
const Eigen::Ref<const plain_type_t<EigMat>>& y_ref = y;
auto&& y_ref = to_ref(value_of_rec(y));
check_lower_triangular(function, name, y_ref);
check_positive(function, name, y_ref.diagonal());
}

/**
* Throw an exception if the specified matrix is not a valid Cholesky factor. A
* Cholesky factor is a lower triangular matrix whose diagonal elements are all
* positive. Note that Cholesky factors need not be square, but require at
* least as many rows M as columns N (i.e., `M >= N`).
* @tparam StdVec A standard vector with inner type either inheriting from
* `MatrixBase` with neither rows or columns defined at compile time to be equal
* to 1 or a `var_value` with the var's inner type inheriting from
* `Eigen::MatrixBase` with neither rows or columns defined at compile time to
* be equal to 1
* @param function Function name (for error messages)
* @param name Variable name (for error messages)
* @param y Standard vector of matrices to test
* @throw `std::domain_error` if y is not a valid Cholesky factor, if number of
* rows is less than the number of columns, if there are 0 columns, or if any
* element in matrix is `NaN`
*/
template <typename StdVec, require_std_vector_t<StdVec>* = nullptr>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[optional]
I'd rather keep shorter template parameters, like just V for standard vectors or maybe C for generic containers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For very generic functions I like using V or T etc. but for functions with specific requirements I like that the template parameter's name gives an idea of what the requirement is to use the function.

void check_cholesky_factor(const char* function, const char* name,
const StdVec& y) {
for (size_t i = 0; i < y.size(); ++i) {
check_cholesky_factor(function, internal::make_iter_name(name, i).c_str(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a performance evaluation as it's going to proactively create string names for each entry, which is pretty expensive.

[question]
Is there a way to be lazy and avoid creating the name until the check fails?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with running a little performance check here, I def expect it to be slower for small matrices though hopefully not much.

Is there a way to be lazy and avoid creating the name until the check fails?

I tried thinking about this but the only thing I could figure out is to change all of the checks to take in a lambda instead of a const char* that doesn't evaluate until a throw occurs.

y[i]);
}
}

} // namespace math
} // namespace stan
#endif
62 changes: 47 additions & 15 deletions stan/math/prim/err/check_cholesky_factor_corr.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,39 @@
#include <stan/math/prim/fun/Eigen.hpp>
#include <stan/math/prim/meta.hpp>
#include <stan/math/prim/fun/to_ref.hpp>
#include <stan/math/prim/fun/value_of_rec.hpp>
#include <stan/math/prim/err/check_positive.hpp>
#include <stan/math/prim/err/check_lower_triangular.hpp>
#include <stan/math/prim/err/check_square.hpp>
#include <stan/math/prim/err/check_unit_vector.hpp>
#include <stan/math/prim/err/make_iter_name.hpp>

namespace stan {
namespace math {

/**
* Check if the specified matrix is a valid Cholesky factor of a
* correlation matrix.
* A Cholesky factor is a lower triangular matrix whose diagonal
* elements are all positive. Note that Cholesky factors need not
* be square, but require at least as many rows M as columns N
* (i.e., M &gt;= N).
* Tolerance is specified by <code>math::CONSTRAINT_TOLERANCE</code>.
* @tparam EigMat Type inheriting from `MatrixBase` with dynamic rows and
* columns.
* Throw an exception if the specified matrix is not a valid Cholesky factor of
* a correlation matrix. A Cholesky factor is a lower triangular matrix whose
* diagonal elements are all positive and each row has unit Euclidean length.
* Note that Cholesky factors need not be square, but require at least as many
* rows M as columns N (i.e., `M >= N`). Tolerance is specified by
* `math::CONSTRAINT_TOLERANCE`. Tolerance is specified by
* `math::CONSTRAINT_TOLERANCE`.
* @tparam Mat Type inheriting from `MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1 or a `var_value` with the var's
* inner type inheriting from `Eigen::MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1
* @param function Function name (for error messages)
* @param name Variable name (for error messages)
* @param y Matrix to test
* @throw <code>std::domain_error</code> if y is not a valid Cholesky
* factor, if number of rows is less than the number of columns,
* if there are 0 columns, or if any element in matrix is NaN
* @throw `std::domain_error` if y is not a valid Cholesky factor, if number of
* rows is less than the number of columns, if there are 0 columns, or if any
* element in matrix is NaN
*/
template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr>
template <typename Mat, require_matrix_t<Mat>* = nullptr>
void check_cholesky_factor_corr(const char* function, const char* name,
const EigMat& y) {
const auto& y_ref = to_ref(y);
const Mat& y) {
const auto& y_ref = to_ref(value_of_rec(y));
check_square(function, name, y_ref);
check_lower_triangular(function, name, y_ref);
check_positive(function, name, y_ref.diagonal());
Expand All @@ -41,6 +45,34 @@ void check_cholesky_factor_corr(const char* function, const char* name,
}
}

/**
* Throw an exception if the specified matrix is not a valid Cholesky factor of
* a correlation matrix. A Cholesky factor is a lower triangular matrix whose
* diagonal elements are all positive and each row has unit Euclidean length.
* Note that Cholesky factors need not be square, but require at least as many
* rows M as columns N (i.e., `M >= N`). Tolerance is specified by
* `math::CONSTRAINT_TOLERANCE`. Tolerance is specified by
* `math::CONSTRAINT_TOLERANCE`.
* @tparam StdVec A standard vector with inner type either inheriting from
* `MatrixBase` with neither rows or columns defined at compile time to be equal
* to 1 or a `var_value` with the var's inner type inheriting from
* `Eigen::MatrixBase` with neither rows or columns defined at compile time to
* be equal to 1
* @param function Function name (for error messages)
* @param name Variable name (for error messages)
* @param y Standard vector of matrics to test
* @throw `std::domain_error` if y[i] is not a valid Cholesky factor, if number
* of rows is less than the number of columns, if there are 0 columns, or if any
* element in matrix is NaN
*/
template <typename StdVec, require_std_vector_t<StdVec>* = nullptr>
void check_cholesky_factor_corr(const char* function, const char* name,
const StdVec& y) {
for (size_t i = 0; i < y.size(); ++i) {
check_cholesky_factor_corr(function,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question for all of these and efficiency.

internal::make_iter_name(name, i).c_str(), y[i]);
}
}
} // namespace math
} // namespace stan
#endif
75 changes: 50 additions & 25 deletions stan/math/prim/err/check_corr_matrix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@

#include <stan/math/prim/fun/Eigen.hpp>
#include <stan/math/prim/meta.hpp>
#include <stan/math/prim/fun/to_ref.hpp>
#include <stan/math/prim/fun/value_of_rec.hpp>
#include <stan/math/prim/err/throw_domain_error.hpp>
#include <stan/math/prim/err/check_pos_definite.hpp>
#include <stan/math/prim/err/check_square.hpp>
#include <stan/math/prim/fun/to_ref.hpp>
#include <sstream>
#include <string>
#include <cmath>
Expand All @@ -15,44 +16,68 @@ namespace stan {
namespace math {

/**
* Check if the specified matrix is a valid correlation matrix.
* A valid correlation matrix is symmetric, has a unit diagonal
* (all 1 values), and has all values between -1 and 1
* (inclusive).
* This function throws exceptions if the variable is not a valid
* correlation matrix.
* @tparam EigMat Type inheriting from `MatrixBase` with dynamic rows and
* columns.
* Throw an exception if the specified matrix is not a valid correlation matrix.
* A valid correlation matrix is symmetric positive definite, has a unit
* diagonal (all 1 values), and has all values between -1 and 1 (inclusive).
* @tparam Mat Type inheriting from `MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1 or a `var_value` with the var's
* inner type inheriting from `Eigen::MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1
* @param function Name of the function this was called from
* @param name Name of the variable
* @param y Matrix to test
* @throw <code>std::invalid_argument</code> if the matrix is not square
* @throw <code>std::domain_error</code> if the matrix is non-symmetric,
* diagonals not near 1, not positive definite, or any of the
* elements nan
* @throw `std::invalid_argument` if the matrix is not square
* @throw `std::domain_error` if the matrix is non-symmetric, diagonals not near
* 1, not positive definite, or any of the elements are `NaN`
*/
template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr>
template <typename Mat, require_matrix_t<Mat>* = nullptr>
inline void check_corr_matrix(const char* function, const char* name,
const EigMat& y) {
const auto& y_ref = to_ref(y);
const Mat& y) {
auto&& y_ref = to_ref(value_of_rec(y));
check_square(function, name, y_ref);
using std::fabs;
if (y_ref.size() == 0) {
return;
}

for (Eigen::Index k = 0; k < y.rows(); ++k) {
if (!(fabs(y_ref(k, k) - 1.0) <= CONSTRAINT_TOLERANCE)) {
std::ostringstream msg;
msg << "is not a valid correlation matrix. " << name << "("
<< stan::error_index::value + k << "," << stan::error_index::value + k
<< ") is ";
std::string msg_str(msg.str());
throw_domain_error(function, name, y_ref(k, k), msg_str.c_str(),
", but should be near 1.0");
if (!(fabs(y_ref.coeff(k, k) - 1.0) <= CONSTRAINT_TOLERANCE)) {
[&y_ref, name, k, function]() STAN_COLD_PATH {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is STAN_COLD_PATH and why is this function being called rather than just executing its body here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See issue #2249 for more info, but essentially STAN_COLD_PATH is equal to __attribute__((noinline,cold)) if noinline and cold are available attributes from the compiler. This makes the error checking cheaper since the compiler knows it very rarely has to go down the path of throwing an error and doesn't pay for prefetching the stuff inside of the if().

std::ostringstream msg;
msg << "is not a valid correlation matrix. " << name << "("
<< stan::error_index::value + k << ","
<< stan::error_index::value + k << ") is ";
std::string msg_str(msg.str());
throw_domain_error(function, name, y_ref(k, k), msg_str.c_str(),
", but should be near 1.0");
}();
}
}
check_pos_definite(function, "y", y_ref);
check_pos_definite(function, name, y_ref);
}

/**
* Throw an exception if the specified matrix is not a valid correlation matrix.
* A valid correlation matrix is symmetric positive definite, has a unit
* diagonal (all 1 values), and has all values between -1 and 1 (inclusive).
* @tparam StdVec A standard vector with inner type either inheriting from
* `Eigen::MatrixBase` with neither rows or columns defined at compile time to
* be equal to 1 or a `var_value` with the var's inner type inheriting from
* `Eigen::MatrixBase` with neither rows or columns defined at compile time to
* be equal to 1.
* @param function Name of the function this was called from
* @param name Name of the variable
* @param y Matrix to test
* @throw `std::invalid_argument` if the matrix is not square
* @throw `std::domain_error` if the matrix is non-symmetric, diagonals not near
* 1, not positive definite, or any of the elements are `NaN`
*/
template <typename StdVec, require_std_vector_t<StdVec>* = nullptr>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a public function and thus should have doc.

void check_corr_matrix(const char* function, const char* name,
const StdVec& y) {
for (auto&& y_i : y) {
check_corr_matrix(function, name, y_i);
}
}

} // namespace math
Expand Down
51 changes: 39 additions & 12 deletions stan/math/prim/err/check_cov_matrix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,58 @@

#include <stan/math/prim/fun/Eigen.hpp>
#include <stan/math/prim/meta.hpp>
#include <stan/math/prim/fun/to_ref.hpp>
#include <stan/math/prim/fun/value_of_rec.hpp>
#include <stan/math/prim/err/check_pos_definite.hpp>

namespace stan {
namespace math {
/**
* Check if the specified matrix is a valid covariance matrix.
* A valid covariance matrix is a square, symmetric matrix that is
* positive definite.
* @tparam EigMat Type inheriting from `MatrixBase` with dynamic rows and
* columns.
* Throw an exception if the specified matrix is not a valid covariance matrix.
* A valid covariance matrix is a square, symmetric matrix that is positive
* definite.
* @tparam Mat Type inheriting from `MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1 or a `var_value` with the var's
* inner type inheriting from `Eigen::MatrixBase` with neither rows or columns
* defined at compile time to be equal to 1
* @param function Function name (for error messages)
* @param name Variable name (for error messages)
* @param y Matrix to test
* @throw <code>std::invalid_argument</code> if the matrix is not square
* or if the matrix is 0x0
* @throw <code>std::domain_error</code> if the matrix is not symmetric,
* if the matrix is not positive definite,
* or if any element of the matrix is nan
* @throw `std::invalid_argument` if the matrix is not square or if the matrix
* is 0x0
* @throw `std::domain_error` if the matrix is not symmetric, if the matrix is
* not positive definite, or if any element of the matrix is `NaN`
*/
template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr>
template <typename Mat, require_matrix_t<Mat>* = nullptr>
inline void check_cov_matrix(const char* function, const char* name,
const EigMat& y) {
const Mat& y) {
check_pos_definite(function, name, y);
}

/**
* Throw an exception if the specified matrix is not a valid covariance matrix.
* A valid covariance matrix is a square, symmetric matrix that is positive
* definite.
* @tparam StdVec A standard vector with inner type either inheriting from
* `MatrixBase` with neither rows or columns defined at compile time to be equal
* to 1 or a `var_value` with the var's inner type inheriting from
* `Eigen::MatrixBase` with neither rows or columns defined at compile time to
* be equal to 1
* @param function Function name (for error messages)
* @param name Variable name (for error messages)
* @param y standard vector of matrices to test.
* @throw `std::invalid_argument` if the matrix is not square
* or if the matrix is 0x0
* @throw `std::domain_error` if the matrix is not symmetric, if the matrix is
* not positive definite, or if any element of the matrix is `NaN`
*/
template <typename StdVec, require_std_vector_t<StdVec>* = nullptr>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs doc.

void check_cov_matrix(const char* function, const char* name, const StdVec& y) {
for (auto&& y_i : y) {
check_cov_matrix(function, name, y_i);
}
}

} // namespace math
} // namespace stan
#endif
Loading