Skip to content
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

Jump to bottom

[WIP] initial implementation of sqrt_spd and inv_sqrt_spd #1145

Closed
wants to merge 11 commits into from
Closed

[WIP] initial implementation of sqrt_spd and inv_sqrt_spd #1145

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
73bf8b3
initial implementation of sqrt_spd and inv_sqrt_spd
bgoodri Mar 7, 2019
f85bae9
Updated the input and output types of the sqrt_spd adj_jac_apply (Iss…
bbbales2 Mar 8, 2019
3f5807a
Merge commit '9d0acc533a2c8656e14bc5db4f4da72ee99cf699' into HEAD
yashikno Mar 8, 2019
5480a40
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot Mar 8, 2019
f89b6a5
fix order of headers
bgoodri Mar 8, 2019
ca80605
bump size of test up
bgoodri Mar 8, 2019
2fa0a27
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot Mar 8, 2019
2b1f2fe
add check for NaNs
bgoodri Mar 8, 2019
1751a76
more usage of adj_jac_apply()
bgoodri Mar 10, 2019
a6daa9e
Merge branch 'feature/matrix_sqrt' of https://github.com/stan-dev/mat…
bgoodri Mar 10, 2019
1adbee1
[Jenkins] auto-formatting by clang-format version 5.0.0-3~16.04.1 (ta…
stan-buildbot Mar 10, 2019
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
Prev Previous commit
Next Next commit
Updated the input and output types of the sqrt_spd adj_jac_apply (Issue 
#1144)

Rearranged some includes in stan/math/rev/mat.hpp to get rid of some type traits errors. Not sure why these were moved around. Might need moved back.
  • Loading branch information
@bbbales2
bbbales2 committed Mar 8, 2019
commit f85bae9deb71b82402eaed01dbe8aa9698bb5598
6 changes: 3 additions & 3 deletions stan/math/rev/mat.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@

#include <stan/math/rev/mat/fun/Eigen_NumTraits.hpp>

#include <stan/math/rev/mat/vectorize/apply_scalar_unary.hpp>
#include <stan/math/prim/mat.hpp>
#include <stan/math/rev/arr.hpp>
#include <stan/math/rev/mat/vectorize/apply_scalar_unary.hpp>

#include <stan/math/rev/mat/fun/LDLT_alloc.hpp>
#include <stan/math/rev/mat/fun/LDLT_factor.hpp>
#include <stan/math/rev/mat/fun/cholesky_decompose.hpp>
#include <stan/math/rev/mat/fun/columns_dot_product.hpp>
#include <stan/math/rev/mat/fun/columns_dot_self.hpp>
Expand All @@ -26,6 +24,8 @@
#include <stan/math/rev/mat/fun/gp_periodic_cov.hpp>
#include <stan/math/rev/mat/fun/grad.hpp>
#include <stan/math/rev/mat/fun/initialize_variable.hpp>
#include <stan/math/rev/mat/fun/LDLT_alloc.hpp>
#include <stan/math/rev/mat/fun/LDLT_factor.hpp>
#include <stan/math/rev/mat/fun/log_determinant.hpp>
#include <stan/math/rev/mat/fun/log_determinant_ldlt.hpp>
#include <stan/math/rev/mat/fun/log_determinant_spd.hpp>
Expand Down
22 changes: 13 additions & 9 deletions stan/math/rev/mat/fun/sqrt_spd.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,23 @@ class sqrt_spd_op {
// derivative-or-differential-of-symmetric-square-root-of-a-matrix

template <std::size_t size>
std::tuple<Eigen::VectorXd>
std::tuple<Eigen::MatrixXd>
multiply_adjoint_jacobian(const std::array<bool, size> &needs_adj,
const Eigen::VectorXd &adj) const {
const Eigen::MatrixXd &adj) const {
bgoodri marked this conversation as resolved.
Show resolved Hide resolved
using Eigen::kroneckerProduct;
using Eigen::MatrixXd;
Eigen::MatrixXd output(K_, K_);
Eigen::Map<const Eigen::VectorXd> map_input(adj.data(), adj.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And if adj is a MatrixXd, how can it be mapped to a VectorXd?

Copy link
Member

Choose a reason for hiding this comment

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

That mapping is doing this (https://en.wikipedia.org/wiki/Vectorization_(mathematics)). I was just pattern-matching the eqs in the Stackoverflow link. I'm not really familiar with that. I was just double checking myself and Abhishek makes a note at the end of his post that the Jacobian he wrote is in terms of the vectorization, which I assume is the Wikipedia definition.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah fair warning, I was just kinda guessing on the math stuff to make things compile. Maybe add in a diagonal matrix test (with things that aren't 1 on the diagonal) so we know the answer and gradients and all just to make sure things are in the right direction.

Eigen::Map<Eigen::VectorXd> map_output(output.data(), output.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar, how can map_output be a Map<VectorXd> but output be a square MatrixXd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And why is map_output necessary? Can we just do the linear algebra to form output and do std::make_tuple(output)?

Copy link
Member

Choose a reason for hiding this comment

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

The maps are just an eigen thing for mapping one data structure onto another. So the math happens in terms of the vectors like Abhishek had it but it just ends up in the right place in the matrices. I think it should be okay in terms of efficiency?

Eigen::Map<MatrixXd> sqrt_m(y_, K_, K_);
return std::make_tuple(
(kroneckerProduct(sqrt_m, MatrixXd::Identity(K_, K_)) +
kroneckerProduct(MatrixXd::Identity(K_, K_), sqrt_m))
// the above is symmetric so can skip the transpose
.ldlt()
.solve(adj)
.eval());

map_output = (kroneckerProduct(sqrt_m, MatrixXd::Identity(K_, K_)) +
kroneckerProduct(MatrixXd::Identity(K_, K_), sqrt_m))
// the above is symmetric so can skip the transpose
.ldlt()
.solve(map_input);

return std::make_tuple(output);
}
};
} // namespace
Expand Down
4 changes: 2 additions & 2 deletions test/unit/math/rev/mat/fun/sqrt_spd_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ TEST(AgradRevMatrix, check_varis_on_stack) {
test::check_varis_on_stack(sqrt_spd(a));
}

/*

struct make_zero {
template <typename T>
Eigen::Matrix<T, Eigen::Dynamic, 1> operator()(
Expand Down Expand Up @@ -57,4 +57,4 @@ TEST(AgradRevMatrix, sqrt_spd) {
if (i != j) EXPECT_NEAR(J(i, j), 0, TOL);
EXPECT_TRUE(J.array().any());
}
*/