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.

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

Add probability tests to check vectorized/scalar lpmfs/lpdfs are the same #1989

Closed
wants to merge 57 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
d6c5225
Added test to check that vectorized lpdfs/lpmfs the same as evaluatin…
bbbales2 Jul 28, 2020
bd2ebc1
Re-indenting code (Issue #1861)
bbbales2 Jul 28, 2020
ea07b9e
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 28, 2020
0aa88c1
Changed how memory is handled to avoid segfaults. Fixed vector handli…
bbbales2 Jul 28, 2020
fdc7757
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 28, 2020
5db698a
Merge commit 'fdf5db851ea6f2c5dc59fcb9e9aa45b24b202afe' into HEAD
yashikno Jul 28, 2020
35b74c4
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 28, 2020
6ebcaad
Finished merge (Issue #1861)
bbbales2 Jul 28, 2020
0ef320e
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 28, 2020
778280f
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 28, 2020
95b4288
Fixed handling of Eigen matrices of fvar<T> types in test framework (…
bbbales2 Jul 29, 2020
a631664
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 29, 2020
f2d7504
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 29, 2020
a080c63
Fixed Frechet distribution for higher order autodiff (Issue #1861)
bbbales2 Jul 29, 2020
84310eb
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 29, 2020
3f2c5ea
Added Frechet test in mix (Issue #1861)
bbbales2 Jul 29, 2020
69045e7
Merge commit 'd34f10a67df9affb3e12af4b7f2a7fd4d6f757d3' into HEAD
yashikno Jul 29, 2020
acb7a3f
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 29, 2020
9304cf9
Switched test framework to use equality checks from unit tests which …
bbbales2 Jul 29, 2020
79231ca
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 29, 2020
e68772a
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 29, 2020
70de131
Fixed Gumbel test distribution implementation (Issue #1861)
bbbales2 Jul 30, 2020
e84ff0a
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 30, 2020
8e4886c
Replaced the comparisons in probability distribution comparisons with…
bbbales2 Jul 30, 2020
791d99c
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 30, 2020
bd880ee
Adjusted tolerances for finite difference comparison (Issue #1861)
bbbales2 Jul 30, 2020
e0277ca
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 30, 2020
f934306
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Jul 30, 2020
1fe965c
Replaced manual finite differences in lpdf + lpmf tests with stan::ma…
bbbales2 Jul 31, 2020
603dc28
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Jul 31, 2020
5732b5e
Changed exponential distribution test case so finite difference gradi…
bbbales2 Aug 1, 2020
025c88a
Changed Von Mises test so finite differences would work (Issue #1861)
bbbales2 Aug 1, 2020
f3eca66
Updated finite difference code to avoid integers (Issue #1861)
bbbales2 Aug 2, 2020
4b3546f
Merge commit '0e80ba3a8c7d98e3624056df860181150274867a' into HEAD
yashikno Aug 2, 2020
b5e234d
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 2, 2020
251b9a7
Fixes for build errors that happen with #1951 merged
bbbales2 Aug 3, 2020
4238841
Merge branch 'bugfix/fix_jumbo_tests' into bugfix/issue-1861-scalars-…
bbbales2 Aug 3, 2020
f62e5b4
Merge remote-tracking branch 'origin/develop' into bugfix/issue-1861-…
bbbales2 Aug 3, 2020
5e1a87b
Added extra tests and stan::test::expect_near_rel and other fixes fro…
bbbales2 Aug 3, 2020
12f59b7
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 4, 2020
f610349
Fixed repeat as vector cdf test (Issue #1861)
bbbales2 Aug 4, 2020
b47308f
Merge commit '7d860336a247ebe3edbac50fb1409f73a9209941' into HEAD
yashikno Aug 4, 2020
3c95343
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 4, 2020
afcf788
Switch EXPECT_FLOAT_EQS to stan::test::expect_near_rel (Issue #1861)
bbbales2 Aug 4, 2020
ff8d6ce
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Aug 4, 2020
dfd1079
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 4, 2020
55d5c8a
Made lots of changes to cdf/lcdf/lccdf tests to bring them in line wi…
bbbales2 Aug 6, 2020
dcb7216
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Aug 6, 2020
52087d6
Merge commit '5b30e403bb1b2276dee2dd3d6736e52e67960651' into HEAD
yashikno Aug 6, 2020
9fe249f
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 6, 2020
4f27b6f
Adding missing using statement (Issue #1861)
bbbales2 Aug 6, 2020
dbdca40
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Aug 6, 2020
fe5c9fd
Changed gamma ccdf test to make the numerics easier (Issue #1861)
bbbales2 Aug 6, 2020
f98703e
Merge commit 'a6dbe55bd908b33b3a98b14dfaae25b269d3161d' into HEAD
yashikno Aug 6, 2020
5bbea21
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Aug 6, 2020
dde875c
Fixed bug introduced in discrete range (Issue #1861)
bbbales2 Aug 7, 2020
6fdf624
Merge branch 'bugfix/issue-1861-scalars-vs-vectors' of https://github…
bbbales2 Aug 7, 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
11 changes: 5 additions & 6 deletions stan/math/prim/prob/discrete_range_cdf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,16 @@ double discrete_range_cdf(const T_y& y, const T_lower& lower,
if (y_dbl < lower_vec[n]) {
return 0;
}
if (y_dbl > upper_vec[n]) {
return 1;
}
}

double cdf(1.0);
for (size_t n = 0; n < N; n++) {
const double y_dbl = y_vec[n];
const double lower_dbl = lower_vec[n];
const double upper_dbl = upper_vec[n];
cdf *= (y_dbl - lower_dbl + 1) / (upper_dbl - lower_dbl + 1);
if (y_dbl < upper_vec[n]) {
Copy link
Member Author

@bbbales2 bbbales2 Aug 10, 2020

Choose a reason for hiding this comment

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

Apologies but for the discrete range changes you're just gonna have to check me (by this I mean check I'm doing the bounds correctly).

This if goes inside this loop because y_dbl > upper_vec multiplies the cdf by 1.0 (the vectorized cdfs are the products of all the constituent cdfs).

If the if statement stays outside, then this function erroneously returns 1 if any single cdf term should be one.

const double lower_dbl = lower_vec[n];
const double upper_dbl = upper_vec[n];
cdf *= (y_dbl - lower_dbl + 1) / (upper_dbl - lower_dbl + 1);
}
}
return cdf;
}
Expand Down
13 changes: 6 additions & 7 deletions stan/math/prim/prob/discrete_range_lccdf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,19 @@ double discrete_range_lccdf(const T_y& y, const T_lower& lower,

for (size_t n = 0; n < N; ++n) {
const int y_dbl = y_vec[n];
if (y_dbl < lower_vec[n]) {
return 0;
}
if (y_dbl > upper_vec[n]) {
if (y_dbl >= upper_vec[n]) {
return LOG_ZERO;
}
}

double ccdf(0.0);
for (size_t n = 0; n < N; n++) {
const int y_dbl = y_vec[n];
const int lower_dbl = lower_vec[n];
const int upper_dbl = upper_vec[n];
ccdf += log(upper_dbl - y_dbl) - log(upper_dbl - lower_dbl + 1);
if (y_dbl >= lower_vec[n]) {
const int lower_dbl = lower_vec[n];
const int upper_dbl = upper_vec[n];
ccdf += log(upper_dbl - y_dbl) - log(upper_dbl - lower_dbl + 1);
}
}
return ccdf;
}
Expand Down
11 changes: 5 additions & 6 deletions stan/math/prim/prob/discrete_range_lcdf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,16 @@ double discrete_range_lcdf(const T_y& y, const T_lower& lower,
if (y_dbl < lower_vec[n]) {
return LOG_ZERO;
}
if (y_dbl > upper_vec[n]) {
return 0;
}
}

double cdf(0.0);
for (size_t n = 0; n < N; n++) {
const int y_dbl = y_vec[n];
const int lower_dbl = lower_vec[n];
const int upper_dbl = upper_vec[n];
cdf += log(y_dbl - lower_dbl + 1) - log(upper_dbl - lower_dbl + 1);
if (y_dbl < upper_vec[n]) {
const int lower_dbl = lower_vec[n];
const int upper_dbl = upper_vec[n];
cdf += log(y_dbl - lower_dbl + 1) - log(upper_dbl - lower_dbl + 1);
}
}
return cdf;
}
Expand Down
7 changes: 3 additions & 4 deletions stan/math/prim/prob/frechet_lpdf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ return_type_t<T_y, T_shape, T_scale> frechet_lpdf(const T_y& y,
T_partials_return, T_y>
inv_y(size(y));
for (size_t i = 0; i < stan::math::size(y); i++) {
inv_y[i] = 1.0 / value_of(y_vec[i]);
inv_y[i] = inv(value_of(y_vec[i]));
}

VectorBuilder<include_summand<propto, T_y, T_shape, T_scale>::value,
Expand All @@ -107,10 +107,9 @@ return_type_t<T_y, T_shape, T_scale> frechet_lpdf(const T_y& y,
logp -= sigma_div_y_pow_alpha[n];

if (!is_constant_all<T_y>::value) {
const T_partials_return inv_y_dbl = value_of(inv_y[n]);
Copy link
Member Author

Choose a reason for hiding this comment

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

The problem here was this second value_of (inv_y has already had one value_of).

ops_partials.edge1_.partials_[n]
+= -(alpha_dbl + 1.0) * inv_y_dbl
+ alpha_dbl * sigma_div_y_pow_alpha[n] * inv_y_dbl;
+= -(alpha_dbl + 1.0) * inv_y[n]
+ alpha_dbl * sigma_div_y_pow_alpha[n] * inv_y[n];
}
if (!is_constant_all<T_shape>::value) {
ops_partials.edge2_.partials_[n]
Expand Down
4 changes: 2 additions & 2 deletions test/prob/binomial/binomial_cdf_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ class AgradCdfBinomial : public AgradCdfTest {
using std::exp;
using std::log;

stan::return_type_t<T_prob> cdf(1);
stan::return_type_t<T_prob> cdf(0);
Copy link
Member Author

Choose a reason for hiding this comment

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


for (int i = 0; i <= n; i++) {
cdf *= binomial_coefficient<double>(N, i)
cdf += binomial_coefficient<double>(N, i)
* exp(i * log(theta) + (N - i) * log(1 - theta));
}

Expand Down
6 changes: 5 additions & 1 deletion test/prob/discrete_range/discrete_range_ccdf_log_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ class AgradCcdfLogDiscreteRange : public AgradCcdfLogTest {
stan::return_type_t<T_y, T_lower, T_upper> ccdf_log_function(
const T_y& y, const T_lower& lower, const T_upper& upper, const T3&,
const T4&, const T5&) {
if (y < lower || y > upper) {
if (y < lower) {
return 1.0;
}

if (y >= upper) {
return stan::math::LOG_ZERO;
}

Expand Down
6 changes: 5 additions & 1 deletion test/prob/discrete_range/discrete_range_cdf_log_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ class AgradCdfLogDiscreteRange : public AgradCdfLogTest {
double cdf_log_function(const T_y& y, const T_lower& lower,
const T_upper& upper, const T3&, const T4&,
const T5&) {
if (y < lower || y > upper) {
if (y < lower) {
return stan::math::LOG_ZERO;
}

if (y > upper) {
return 0.0;
}

return log((y - lower + 1.0) / (upper - lower + 1.0));
}
};
6 changes: 5 additions & 1 deletion test/prob/discrete_range/discrete_range_cdf_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,14 @@ class AgradCdfDiscreteRange : public AgradCdfTest {
typename T5>
double cdf_function(const T_y& y, const T_lower& lower, const T_upper& upper,
const T3&, const T4&, const T5&) {
if (y < lower || y > upper) {
if (y < lower) {
return 0;
}

if (y > upper) {
return 1;
}

return (y - lower + 1.0) / (upper - lower + 1.0);
}
};
4 changes: 2 additions & 2 deletions test/prob/exponential/exponential_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ class AgradDistributionsExponential : public AgradDistributionTest {
parameters.push_back(param);
log_prob.push_back(-57.13902344686439249699); // expected log_prob

param[0] = 1e-08;
param[0] = 1e-2;
Copy link
Member Author

Choose a reason for hiding this comment

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

This test point needs to be far enough away from zero so that the finite difference checks don't go out of range.

param[1] = 3.9;
parameters.push_back(param);
log_prob.push_back(1.3609765141356007);
log_prob.push_back(1.3219765531356007);
}

void invalid_values(vector<size_t>& index, vector<double>& value) {
Expand Down
1 change: 0 additions & 1 deletion test/prob/frechet/frechet_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class AgradDistributionsFrechet : public AgradDistributionTest {
const T4&, const T5&) {
using stan::math::include_summand;
using stan::math::multiply_log;
using stan::math::value_of;
using std::log;
using std::pow;

Expand Down
16 changes: 8 additions & 8 deletions test/prob/gamma/gamma_ccdf_log_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ class AgradCcdfLogGamma : public AgradCcdfLogTest {
ccdf_log.push_back(
std::log(1.0 - 0.6321205588285576659757)); // expected ccdf_log

param[0] = 16.0; // y
param[1] = 3.0; // alpha
param[2] = 3.0; // beta
param[0] = 9.0; // y
Copy link
Member Author

Choose a reason for hiding this comment

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

I made the test case easier because the test implementations were struggling numerically. See: #2006

The Stan implementations of the ccdf didn't seem to have this problem.

param[1] = 1.2; // alpha
param[2] = 1.2; // beta
parameters.push_back(param);
ccdf_log.push_back(std::log(1.7116220633718619e-18)); // expected ccdf_log
ccdf_log.push_back(-10.221534317077268); // expected ccdf_log
}

void invalid_values(vector<size_t>& index, vector<double>& value) {
Expand Down Expand Up @@ -90,10 +90,10 @@ class AgradCcdfLogGamma : public AgradCcdfLogTest {
stan::return_type_t<T_y, T_shape, T_inv_scale> ccdf_log_function(
const T_y& y, const T_shape& alpha, const T_inv_scale& beta, const T3&,
const T4&, const T5&) {
using boost::math::gamma_q;
using stan::math::gamma_q;
using std::log;
using boost::math::gamma_p;
using stan::math::gamma_p;
using stan::math::log1m;

return log(gamma_q(alpha, beta * y));
return log1m(gamma_p(alpha, beta * y));
Copy link
Member Author

Choose a reason for hiding this comment

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

Doing log1m(cdf) worked better than log(ccdf) for forward mode. Check here for the lcdf: https://github.com/stan-dev/math/blob/develop/test/prob/gamma/gamma_cdf_log_test.hpp#L84

}
};
2 changes: 1 addition & 1 deletion test/prob/gumbel/gumbel_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ class AgradDistributionGumbel : public AgradDistributionTest {
stan::return_type_t<T_y, T_loc, T_scale> log_prob_function(
const T_y& y, const T_loc& mu, const T_scale& beta, const T3&, const T4&,
const T5&) {
return -log(beta) - (y - mu) / beta + exp((mu - y) / beta);
return -log(beta) - (y - mu) / beta - exp((mu - y) / beta);
Copy link
Member Author

Choose a reason for hiding this comment

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

}
};
2 changes: 1 addition & 1 deletion test/prob/inv_chi_square/inv_chi_square_ccdf_log_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ class AgradCcdfLogInvChiSquare : public AgradCcdfLogTest {
const T5&) {
using stan::math::gamma_p;

return log(1.0 - gamma_p(0.5 * nu, 0.5 / y));
return log(gamma_p(0.5 * nu, 0.5 / y));
Copy link
Member Author

Choose a reason for hiding this comment

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

Here's the math version: https://github.com/stan-dev/math/blob/develop/stan/math/prim/prob/inv_chi_square_lccdf.hpp#L92

(this is checked with finite differences so I assume it's the correct one)

}
};
Loading