Skip to content

Changed calculation of log_sum_exp(x1, x2) #1772

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 14 commits into from
Mar 19, 2020
Merged

Changed calculation of log_sum_exp(x1, x2) #1772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5fd8e2b
change derivative calculation for log_sum_exp; fixes #1679
pgree Mar 10, 2020
0869a16
remove white space
pgree Mar 10, 2020
2f5432f
remove more white space
pgree Mar 10, 2020
8aa6a50
Merge commit '114985aaf0798584525c0cf7fd6cab0168222f54' into HEAD
yashikno Mar 10, 2020
b513358
[Jenkins] auto-formatting by clang-format version 5.0.0-3~16.04.1 (ta…
stan-buildbot Mar 10, 2020
3a03bd9
replaced calculate_chain and removed function
pgree Mar 11, 2020
088ee0c
Merge remote-tracking branch 'pgree/bugfix/1679-log-sum-exp' into bug…
pgree Mar 11, 2020
817dec8
included inv_logit, added new test
pgree Mar 13, 2020
67a1b13
Merge commit 'fe3a41c3e854fe604841b237fa0475f26d29fe98' into HEAD
yashikno Mar 13, 2020
829be7f
[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…
stan-buildbot Mar 13, 2020
c261931
added test for var and double
pgree Mar 13, 2020
197831a
merged with current
pgree Mar 13, 2020
62a3c1f
updated header
pgree Mar 16, 2020
a793873
updated another header
pgree Mar 16, 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
1 change: 0 additions & 1 deletion stan/math/rev/fun.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <stan/math/rev/fun/bessel_second_kind.hpp>
#include <stan/math/rev/fun/beta.hpp>
#include <stan/math/rev/fun/binary_log_loss.hpp>
#include <stan/math/rev/fun/calculate_chain.hpp>
#include <stan/math/rev/fun/cbrt.hpp>
#include <stan/math/rev/fun/ceil.hpp>
#include <stan/math/rev/fun/cholesky_decompose.hpp>
Expand Down
16 changes: 0 additions & 16 deletions stan/math/rev/fun/calculate_chain.hpp
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions stan/math/rev/fun/log1p_exp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

#include <stan/math/rev/meta.hpp>
#include <stan/math/rev/core.hpp>
#include <stan/math/rev/fun/calculate_chain.hpp>
#include <stan/math/prim/fun/inv_logit.hpp>
#include <stan/math/prim/fun/log1p_exp.hpp>

namespace stan {
Expand All @@ -13,7 +13,7 @@ namespace internal {
class log1p_exp_v_vari : public op_v_vari {
public:
explicit log1p_exp_v_vari(vari* avi) : op_v_vari(log1p_exp(avi->val_), avi) {}
void chain() { avi_->adj_ += adj_ * calculate_chain(avi_->val_, val_); }
void chain() { avi_->adj_ += adj_ * inv_logit(avi_->val_); }
};
} // namespace internal

Expand Down
5 changes: 2 additions & 3 deletions stan/math/rev/fun/log_diff_exp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#include <stan/math/rev/meta.hpp>
#include <stan/math/rev/core.hpp>
#include <stan/math/rev/fun/calculate_chain.hpp>
#include <stan/math/prim/fun/constants.hpp>
#include <stan/math/prim/fun/expm1.hpp>
#include <stan/math/prim/fun/log_diff_exp.hpp>
Expand All @@ -17,7 +16,7 @@ class log_diff_exp_vv_vari : public op_vv_vari {
log_diff_exp_vv_vari(vari* avi, vari* bvi)
: op_vv_vari(log_diff_exp(avi->val_, bvi->val_), avi, bvi) {}
void chain() {
avi_->adj_ += adj_ * calculate_chain(avi_->val_, val_);
avi_->adj_ -= adj_ / expm1(bvi_->val_ - avi_->val_);
Copy link
Member

Choose a reason for hiding this comment

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

There's no way to use inv_logit here? If so, that's fine, just checking.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see how you could because one has the extra 1 term inside and the other outside.

1 / expm1(x) = 1 / exp(x - 1)
inv_logit(u) = 1 / (1 + exp(-u))

Copy link
Member

Choose a reason for hiding this comment

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

Oh good point. I was too lazy to actually think through the math at all.

bvi_->adj_ -= adj_ / expm1(avi_->val_ - bvi_->val_);
}
};
Expand All @@ -29,7 +28,7 @@ class log_diff_exp_vd_vari : public op_vd_vari {
if (val_ == NEGATIVE_INFTY) {
avi_->adj_ += (bd_ == NEGATIVE_INFTY) ? adj_ : adj_ * INFTY;
} else {
avi_->adj_ += adj_ * calculate_chain(avi_->val_, val_);
avi_->adj_ -= adj_ / expm1(bd_ - avi_->val_);
}
}
};
Expand Down
8 changes: 4 additions & 4 deletions stan/math/rev/fun/log_sum_exp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

#include <stan/math/rev/meta.hpp>
#include <stan/math/rev/core.hpp>
#include <stan/math/rev/fun/calculate_chain.hpp>
#include <stan/math/rev/fun/typedefs.hpp>
#include <stan/math/prim/meta.hpp>
#include <stan/math/prim/fun/constants.hpp>
#include <stan/math/prim/fun/Eigen.hpp>
#include <stan/math/prim/fun/inv_logit.hpp>
#include <stan/math/prim/fun/log_sum_exp.hpp>
#include <cmath>
#include <vector>
Expand All @@ -22,8 +22,8 @@ class log_sum_exp_vv_vari : public op_vv_vari {
log_sum_exp_vv_vari(vari* avi, vari* bvi)
: op_vv_vari(log_sum_exp(avi->val_, bvi->val_), avi, bvi) {}
void chain() {
avi_->adj_ += adj_ * calculate_chain(avi_->val_, val_);
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to get rid of calculate_chain everywhere? It seems like a strange function.

Also used here (which should be fixed as part of this pull):

math/stan/math/rev/fun/log_sum_exp.hpp

Line 37 in b513358

avi_->adj_ += adj_ * calculate_chain(avi_->val_, val_);

In stan/math/rev/fun/log_diff_exp.hpp and in stan/math/rev/fun/log1p_exp.hpp.

bvi_->adj_ += adj_ * calculate_chain(bvi_->val_, val_);
avi_->adj_ += adj_ * inv_logit(avi_->val_ - bvi_->val_);
bvi_->adj_ += adj_ * inv_logit(bvi_->val_ - avi_->val_);
}
};
class log_sum_exp_vd_vari : public op_vd_vari {
Expand All @@ -34,7 +34,7 @@ class log_sum_exp_vd_vari : public op_vd_vari {
if (val_ == NEGATIVE_INFTY) {
avi_->adj_ += adj_;
} else {
avi_->adj_ += adj_ * calculate_chain(avi_->val_, val_);
avi_->adj_ += adj_ * inv_logit(avi_->val_ - bd_);
}
}
};
Expand Down
63 changes: 63 additions & 0 deletions test/unit/math/rev/fun/log_sum_exp_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#include <stan/math/rev.hpp>
#include <gtest/gtest.h>
#include <test/unit/math/rev/fun/util.hpp>
#include <test/unit/math/rev/util.hpp>

TEST(log_sum_exp_tests, large_values) {
using stan::math::var;

// check autodiffing works with var types with large values
var a = 1e50;
var output = stan::math::log_sum_exp(a, a);
output.grad();
EXPECT_FLOAT_EQ(output.val(), log(2.0) + value_of(a));
EXPECT_FLOAT_EQ(a.adj(), 1.0);

var a2 = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Once you remove the other calculate_chain thing we'll need some tests where a1 is a var and a2 is a double.

var a3 = 1e50;
var output2 = stan::math::log_sum_exp(a2, a3);
output2.grad();
EXPECT_FLOAT_EQ(a2.adj(), 0.0);
EXPECT_FLOAT_EQ(a3.adj(), 1.0);

var a4 = 1e50;
var a5 = 1;
var output3 = stan::math::log_sum_exp(a4, a5);
output3.grad();
EXPECT_FLOAT_EQ(a4.adj(), 1.0);
EXPECT_FLOAT_EQ(a5.adj(), 0.0);

// check autodiffing works with var types with large values
var b = 1e20;
var output6 = stan::math::log_sum_exp(b, b);
output6.grad();
EXPECT_FLOAT_EQ(output6.val(), log(2.0) + value_of(b));
EXPECT_FLOAT_EQ(b.adj(), 1.0);

var b2 = -2;
var b3 = 1e20;
var output7 = stan::math::log_sum_exp(b2, b3);
output7.grad();
EXPECT_FLOAT_EQ(b2.adj(), 0.0);
EXPECT_FLOAT_EQ(b3.adj(), 1.0);

var b4 = 1e20;
var b5 = -2;
var output8 = stan::math::log_sum_exp(b4, b5);
output8.grad();
EXPECT_FLOAT_EQ(b4.adj(), 1.0);
EXPECT_FLOAT_EQ(b5.adj(), 0.0);

// check arguement combinations of vars and doubles
var a6 = 1e50;
double a7 = 1;
var output4 = stan::math::log_sum_exp(a6, a7);
output4.grad();
EXPECT_FLOAT_EQ(a6.adj(), 1.0);

var a8 = 1;
double a9 = 1e50;
var output5 = stan::math::log_sum_exp(a8, a9);
output5.grad();
EXPECT_FLOAT_EQ(a8.adj(), 0.0);
}