Skip to content

fix returned expressions #1644

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.

Sign up for GitHub

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 13 commits into from
Jan 29, 2020
Merged

fix returned expressions #1644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
70d9c21
fixes returned expressions
t4c1 Jan 27, 2020
58586ef
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot Jan 27, 2020
b8da27c
another case
t4c1 Jan 27, 2020
88cd965
fix minus
t4c1 Jan 27, 2020
dfc4384
added a test.
t4c1 Jan 28, 2020
1cf5579
added remaining tests
t4c1 Jan 28, 2020
b7e741b
fix copy-paste
t4c1 Jan 28, 2020
5b846f6
fix missing includes
t4c1 Jan 28, 2020
1ab15c7
another one
t4c1 Jan 28, 2020
7f277d6
one missed instance of the bug
t4c1 Jan 28, 2020
f821a72
another one
t4c1 Jan 28, 2020
5f7b2a9
fixed atanh input values
t4c1 Jan 28, 2020
9689cb6
fix some more test input values
t4c1 Jan 29, 2020
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/fun/acos.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto acos(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto acos(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().acos().matrix();
return x.derived().array().acos().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/asin.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto asin(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto asin(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().asin().matrix();
return x.derived().array().asin().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/atan.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline typename apply_scalar_unary<atan_fun, T>::return_t atan(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto atan(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().atan().matrix();
return x.derived().array().atan().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/ceil.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto ceil(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto ceil(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().ceil().matrix();
return x.derived().array().ceil().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/cos.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto cos(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto cos(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().cos().matrix();
return x.derived().array().cos().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/cosh.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline typename apply_scalar_unary<cosh_fun, T>::return_t cosh(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto cosh(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().cosh().matrix();
return x.derived().array().cosh().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/exp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ inline auto exp(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto exp(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().exp().matrix();
return x.derived().array().exp().matrix().eval();
}

} // namespace math
Expand Down
4 changes: 2 additions & 2 deletions stan/math/prim/fun/fabs.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline typename apply_scalar_unary<fabs_fun, T>::return_t fabs(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto fabs(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().abs().matrix();
return x.derived().array().abs().matrix().eval();
}

/**
Expand All @@ -57,7 +57,7 @@ inline auto fabs(const Eigen::MatrixBase<Derived>& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto fabs(const Eigen::ArrayBase<Derived>& x) {
return x.derived().abs();
return x.derived().abs().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/floor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto floor(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto floor(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().floor().matrix();
return x.derived().array().floor().matrix().eval();
}

} // namespace math
Expand Down
4 changes: 2 additions & 2 deletions stan/math/prim/fun/inv.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto inv(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto inv(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().inverse().matrix();
return x.derived().array().inverse().matrix().eval();
}

/**
Expand All @@ -57,7 +57,7 @@ inline auto inv(const Eigen::MatrixBase<Derived>& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto inv(const Eigen::ArrayBase<Derived>& x) {
return x.derived().inverse();
return x.derived().inverse().eval();
}

} // namespace math
Expand Down
4 changes: 2 additions & 2 deletions stan/math/prim/fun/inv_cloglog.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ inline auto inv_cloglog(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto inv_cloglog(const Eigen::MatrixBase<Derived>& x) {
return (1 - exp(-exp(x.derived().array()))).matrix();
return (1 - exp(-exp(x.derived().array()))).matrix().eval();
}

/**
Expand All @@ -99,7 +99,7 @@ inline auto inv_cloglog(const Eigen::MatrixBase<Derived>& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto inv_cloglog(const Eigen::ArrayBase<Derived>& x) {
return 1 - exp(-exp(x.derived()));
return (1 - exp(-exp(x.derived()))).eval();
}

} // namespace math
Expand Down
4 changes: 2 additions & 2 deletions stan/math/prim/fun/inv_sqrt.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ inline auto inv_sqrt(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto inv_sqrt(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().rsqrt().matrix();
return x.derived().array().rsqrt().matrix().eval();
}

/**
Expand All @@ -62,7 +62,7 @@ inline auto inv_sqrt(const Eigen::MatrixBase<Derived>& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto inv_sqrt(const Eigen::ArrayBase<Derived>& x) {
return x.derived().rsqrt();
return x.derived().rsqrt().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/log.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ inline auto log(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto log(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().log().matrix();
return x.derived().array().log().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/log10.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto log10(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto log10(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().log10().matrix();
return x.derived().array().log10().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/log_softmax.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ template <typename T, require_t<std::is_arithmetic<scalar_type_t<T>>>...>
inline auto log_softmax(const T& x) {
return apply_vector_unary<T>::apply(x, [&](const auto& v) {
check_nonzero_size("log_softmax", "v", v);
return (v.array() - log_sum_exp(v)).matrix();
return (v.array() - log_sum_exp(v)).matrix().eval();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than adding .eval() to each of these functions, you could instead modify the apply_vector_unary header to eval the eigen returns:

  template <typename F>
  static inline auto apply(const T& x, const F& f) {
    return f(x).eval();
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went this way, so we don't need to first eval and than copy matrices in std::vector overloads that assign result of apply_vector_unary::apply() instead of returning it.

});
}
} // namespace math
Expand Down
4 changes: 3 additions & 1 deletion stan/math/prim/fun/minus.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#ifndef STAN_MATH_PRIM_FUN_MINUS_HPP
#define STAN_MATH_PRIM_FUN_MINUS_HPP

#include <stan/math/prim/meta.hpp>

namespace stan {
namespace math {

Expand All @@ -12,7 +14,7 @@ namespace math {
* @return Negation of subtrahend.
*/
template <typename T>
inline auto minus(const T& x) {
inline plain_type_t<T> minus(const T& x) {
return -x;
}

Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/round.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ inline auto round(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto round(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().round().matrix();
return x.derived().array().round().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/sin.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto sin(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto sin(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().sin().matrix();
return x.derived().array().sin().matrix().eval();
}

} // namespace math
Expand Down
4 changes: 2 additions & 2 deletions stan/math/prim/fun/sinh.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto sinh(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto sinh(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().sinh().matrix();
return x.derived().array().sinh().matrix().eval();
}

/**
Expand All @@ -57,7 +57,7 @@ inline auto sinh(const Eigen::MatrixBase<Derived>& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto sinh(const Eigen::ArrayBase<Derived>& x) {
return x.derived().sinh();
return x.derived().sinh().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/sqrt.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ inline auto sqrt(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto sqrt(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().sqrt().matrix();
return x.derived().array().sqrt().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/tan.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto tan(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto tan(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().tan().matrix();
return x.derived().array().tan().matrix().eval();
}

} // namespace math
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/fun/tanh.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ inline auto tanh(const T& x) {
template <typename Derived,
typename = require_eigen_vt<std::is_arithmetic, Derived>>
inline auto tanh(const Eigen::MatrixBase<Derived>& x) {
return x.derived().array().tanh().matrix();
return x.derived().array().tanh().matrix().eval();
}

} // namespace math
Expand Down
7 changes: 5 additions & 2 deletions stan/math/prim/vectorize/apply_scalar_unary.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ struct apply_scalar_unary<F, T, require_eigen_t<T>> {
* by F to the specified matrix.
*/
static inline auto apply(const T& x) {
return x.unaryExpr(
[](scalar_t x) { return apply_scalar_unary<F, scalar_t>::apply(x); });
return x
.unaryExpr([](scalar_t x) {
return apply_scalar_unary<F, scalar_t>::apply(x);
})
.eval();
}

/**
Expand Down
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/Phi_approx_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ TEST(MathFunctions, Phi_approx_nan) {

EXPECT_TRUE(std::isnan(stan::math::Phi_approx(nan)));
}

TEST(MathFunctions, Phi_approx__works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::Phi_approx(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/Phi_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,11 @@ TEST(MathFunctions, Phi_nan) {
double nan = std::numeric_limits<double>::quiet_NaN();
EXPECT_THROW(stan::math::Phi(nan), std::domain_error);
}

TEST(MathFunctions, Phi_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::Phi(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/acos_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@ TEST(primScalFun, acos) {
stan::test::expect_common_prim([](auto x) { return std::acos(x); },
[](auto x) { return stan::math::acos(x); });
}

TEST(MathFunctions, acos_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 0.1, 0.2, 0.3, 0.4, 0.5;
Eigen::RowVectorXd b(5);
b << 0.1, 0.2, 0.3, 0.4, 0.5;
stan::math::multiply(a, stan::math::acos(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/acosh_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,11 @@ TEST(MathFunctions, acosh_nan) {
double nan = std::numeric_limits<double>::quiet_NaN();
EXPECT_TRUE(std::isnan(stan::math::acosh(nan)));
}

TEST(MathFunctions, acosh_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::acosh(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/asin_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@ TEST(primScalFun, asin) {
stan::test::expect_common_prim([](auto x) { return std::asin(x); },
[](auto x) { return stan::math::asin(x); });
}

TEST(MathFunctions, asin_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 0.1, 0.2, 0.3, 0.4, 0.5;
Eigen::RowVectorXd b(5);
b << 0.1, 0.2, 0.3, 0.4, 0.5;
stan::math::multiply(a, stan::math::asin(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/asinh_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,11 @@ TEST(MathFunctions, asinh_nan) {
double nan = std::numeric_limits<double>::quiet_NaN();
EXPECT_TRUE(std::isnan(stan::math::asinh(nan)));
}

TEST(MathFunctions, asinh_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::asinh(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/atan_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@ TEST(primScalFun, atan) {
stan::test::expect_common_prim([](auto x) { return std::atan(x); },
[](auto x) { return stan::math::atan(x); });
}

TEST(MathFunctions, atan_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::atan(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/atanh_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,11 @@ TEST(MathFunctions, atanh_nan) {
double nan = std::numeric_limits<double>::quiet_NaN();
EXPECT_TRUE(std::isnan(stan::math::atanh(nan)));
}

TEST(MathFunctions, atanh_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 0.1, 0.2, 0.3, 0.4, 0.5;
Eigen::RowVectorXd b(5);
b << 0.1, 0.2, 0.3, 0.4, 0.5;
stan::math::multiply(a, stan::math::atanh(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/cbrt_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,11 @@ TEST(MathFunctions, cbrt_nan) {
double nan = std::numeric_limits<double>::quiet_NaN();
EXPECT_TRUE(std::isnan(stan::math::cbrt(nan)));
}

TEST(MathFunctions, cbrt_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::cbrt(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/ceil_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,11 @@ TEST(primScalFun, ceil) {
stan::test::expect_common_prim([](auto x) { return std::ceil(x); },
[](auto x) { return stan::math::ceil(x); });
}

TEST(MathFunctions, ceil_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::ceil(b));
}
8 changes: 8 additions & 0 deletions test/unit/math/prim/fun/cos_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@ TEST(primScalFun, cos) {
stan::test::expect_common_prim([](auto x) { return std::cos(x); },
[](auto x) { return stan::math::cos(x); });
}

TEST(MathFunctions, cos_works_with_other_functions) {
Eigen::VectorXd a(5);
a << 1.1, 1.2, 1.3, 1.4, 1.5;
Eigen::RowVectorXd b(5);
b << 1.1, 1.2, 1.3, 1.4, 1.5;
stan::math::multiply(a, stan::math::cos(b));
}
Loading