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

add prim and rev gp_exp_cov #859

Closed
wants to merge 16 commits into from
Closed

add prim and rev gp_exp_cov #859

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
098613d
add prim and rev gp_exp_cov
May 9, 2018
bd69dc2
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot May 9, 2018
f9a960b
Update gp_exponential_cov.hpp
May 9, 2018
3df0bc9
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot May 9, 2018
e46bd66
add rev mat.hpp
May 9, 2018
77e77da
temp
May 11, 2018
5212d97
temp
May 11, 2018
26ca567
use ChainableStack::instance()
May 11, 2018
1cc69b9
Merge commit '5cd004ca651c13d63a460c62b86c8a366a8fe830' into HEAD
yashikno May 11, 2018
644d11d
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot May 11, 2018
13b7475
use ChainableStack::instance()
May 11, 2018
47b2805
fix merge conflix
May 11, 2018
d565a9e
proper includes
May 11, 2018
67c0557
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot May 11, 2018
8599e40
working updates for PR review
May 12, 2018
7a4ea7b
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot May 12, 2018
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
proper includes
  • Loading branch information
andre committed May 11, 2018
commit d565a9e12bb88739152514ce0f04527fc97d541c
55 changes: 25 additions & 30 deletions stan/math/rev/mat/fun/gp_exponential_cov.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@
#include <boost/math/tools/promotion.hpp>
#include <boost/type_traits.hpp>
#include <boost/utility/enable_if.hpp>
#include <stan/math/prim/mat/fun/Eigen.hpp>
#include <stan/math/prim/scal/fun/exp.hpp>
#include <stan/math/prim/scal/fun/square.hpp>
#include <stan/math/prim/scal/fun/squared_distance.hpp>
#include <stan/math/prim/scal/err/check_not_nan.hpp>
#include <stan/math/prim/scal/err/check_positive.hpp>
#include <stan/math/prim/scal/meta/scalar_type.hpp>
#include <stan/math/rev/core.hpp>
#include <stan/math/rev/scal/fun/value_of.hpp>
#include <vector>
Expand All @@ -27,7 +32,7 @@ namespace math {
*/
template <typename T_x, typename T_s, typename T_l>
Copy link
Member

Choose a reason for hiding this comment

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

Questions:

  1. any reason this is templated? It looks like the code only has one possible rev instantiation: <double, var, var>.
  2. what's with the other two specializations? Just based on inspecting the code, it looks like there's going to be trouble with template argument deduction. It's got to be ambiguous the way it's written: <T_x, double, T_l> and <T_x, T_s, double>. I guess I'm saying I don't believe that it'll compile if you actually tried to have both in there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why are there two specializations in stan/math/rev/mat/fun/cov_exp_quad.hpp? OK, let me find argument deductions where it fails, and I'll most likely remove the other two specializations.

Copy link
Member

Choose a reason for hiding this comment

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

There are two different partial template specializations in the rev version of cov_exp_quad.

L215 has: cov_exp_quad<double, var, var>
L257 has: cov_exp_quad<double, double, var>

That's why there is a base template function and then a partial specialization of class cov_exp_quad_vari. Now that I look at it, there's probably a different way to implement it, but that's a different question.

class gp_exponential_cov_vari : public vari {
public:
public:
const size_t size_;
const size_t size_ltri_;
const double l_d_;
Expand Down Expand Up @@ -58,16 +63,12 @@ class gp_exponential_cov_vari : public vari {
*/
gp_exponential_cov_vari(const std::vector<T_x> &x, const T_s &sigma,
const T_l &length_scale)
: vari(0.0),
size_(x.size()),
size_ltri_(size_ * (size_ - 1) / 2),
l_d_(value_of(length_scale)),
sigma_d_(value_of(sigma)),
: vari(0.0), size_(x.size()), size_ltri_(size_ * (size_ - 1) / 2),
l_d_(value_of(length_scale)), sigma_d_(value_of(sigma)),
sigma_sq_d_(sigma_d_ * sigma_d_),
dist_(ChainableStack::instance().memalloc_.alloc_array<double>(
Copy link
Member

Choose a reason for hiding this comment

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

dist_ really need to go in the arena memory. We should initialize a std::vector<double> with that size. (I realize the other kernel does that; I think we shouldn't here and there. Let's start with this one.)

size_ltri_)),
l_vari_(length_scale.vi_),
sigma_vari_(sigma.vi_),
l_vari_(length_scale.vi_), sigma_vari_(sigma.vi_),
cov_lower_(ChainableStack::instance().memalloc_.alloc_array<vari *>(
size_ltri_)),
cov_diag_(
Expand All @@ -78,8 +79,8 @@ class gp_exponential_cov_vari : public vari {
for (size_t i = j + 1; i < size_; ++i) {
double dist_sq = squared_distance(x[i], x[j]);
dist_[pos] = dist_sq;
cov_lower_[pos]
= new vari(sigma_sq_d_ * exp(dist_sq * neg_inv_l), false);
cov_lower_[pos] =
new vari(sigma_sq_d_ * exp(dist_sq * neg_inv_l), false);
++pos;
}
}
Expand Down Expand Up @@ -117,7 +118,7 @@ class gp_exponential_cov_vari : public vari {
*/
template <typename T_x, typename T_l>
class gp_exponential_cov_vari<T_x, double, T_l> : public vari {
public:
public:
const size_t size_;
const size_t size_ltri_;
const double l_d_;
Expand Down Expand Up @@ -148,11 +149,8 @@ class gp_exponential_cov_vari<T_x, double, T_l> : public vari {
*/
gp_exponential_cov_vari(const std::vector<T_x> &x, double sigma,
Copy link
Member

Choose a reason for hiding this comment

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

This isn't used.

const T_l &length_scale)
: vari(0.0),
size_(x.size()),
size_ltri_(size_ * (size_ - 1) / 2),
l_d_(value_of(length_scale)),
sigma_d_(value_of(sigma)),
: vari(0.0), size_(x.size()), size_ltri_(size_ * (size_ - 1) / 2),
l_d_(value_of(length_scale)), sigma_d_(value_of(sigma)),
sigma_sq_d_(sigma_d_ * sigma_d_),
dist_(ChainableStack::instance().memalloc_.alloc_array<double>(
size_ltri_)),
Expand All @@ -167,8 +165,8 @@ class gp_exponential_cov_vari<T_x, double, T_l> : public vari {
for (size_t i = j + 1; i < size_; ++i) {
double dist_sq = squared_distance(x[i], x[j]);
dist_[pos] = dist_sq;
cov_lower_[pos]
= new vari(sigma_sq_d_ * exp(dist_sq * neg_inv_l), false);
cov_lower_[pos] =
new vari(sigma_sq_d_ * exp(dist_sq * neg_inv_l), false);
++pos;
}
}
Expand Down Expand Up @@ -199,7 +197,7 @@ class gp_exponential_cov_vari<T_x, double, T_l> : public vari {
*/
template <typename T_x, typename T_s>
Copy link
Member

Choose a reason for hiding this comment

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

This isn't used, so remove.

class gp_exponential_cov_vari<T_x, T_s, double> : public vari {
public:
public:
const size_t size_;
const size_t size_ltri_;
const double l_d_;
Expand Down Expand Up @@ -230,11 +228,8 @@ class gp_exponential_cov_vari<T_x, T_s, double> : public vari {
*/
gp_exponential_cov_vari(const std::vector<T_x> &x, const T_s &sigma,
double length_scale)
: vari(0.0),
size_(x.size()),
size_ltri_(size_ * (size_ - 1) / 2),
l_d_(value_of(length_scale)),
sigma_d_(value_of(sigma)),
: vari(0.0), size_(x.size()), size_ltri_(size_ * (size_ - 1) / 2),
l_d_(value_of(length_scale)), sigma_d_(value_of(sigma)),
sigma_sq_d_(sigma_d_ * sigma_d_),
dist_(ChainableStack::instance().memalloc_.alloc_array<double>(
size_ltri_)),
Expand All @@ -248,8 +243,8 @@ class gp_exponential_cov_vari<T_x, T_s, double> : public vari {
for (size_t i = j + 1; i < size_; ++i) {
double dist_sq = squared_distance(x[i], x[j]);
dist_[pos] = dist_sq;
cov_lower_[pos]
= new vari(sigma_sq_d_ * exp(dist_sq * neg_inv_l), false);
cov_lower_[pos] =
new vari(sigma_sq_d_ * exp(dist_sq * neg_inv_l), false);
++pos;
}
}
Expand Down Expand Up @@ -296,8 +291,8 @@ gp_exponential_cov(const std::vector<T_x> &x, const var &sigma, const var &l) {
if (x_size == 0)
return cov;

gp_exponential_cov_vari<T_x, var, var> *baseVari
= new gp_exponential_cov_vari<T_x, var, var>(x, sigma, l);
gp_exponential_cov_vari<T_x, var, var> *baseVari =
new gp_exponential_cov_vari<T_x, var, var>(x, sigma, l);

size_t pos = 0;
for (size_t j = 0; j < x_size - 1; ++j) {
Expand All @@ -311,7 +306,7 @@ gp_exponential_cov(const std::vector<T_x> &x, const var &sigma, const var &l) {
cov.coeffRef(x_size - 1, x_size - 1).vi_ = baseVari->cov_diag_[x_size - 1];
return cov;
}
} // namespace math
} // namespace stan
} // namespace math
} // namespace stan

#endif