Adds simple vectorized functions for var<matrix> #2461

SteveBronder · 2021-04-07T22:02:50Z

Summary

This adds a bunch of var versions of functions that were in #2422 . I stopped about halfway though so the PR didn't get too big.

Tests

Tests added for var equivalent functions.

./runTests.py test/unit/math/mix/fun/bessel_first_kind_test.cpp \
test/unit/math/mix/fun/bessel_second_kind_test.cpp \
test/unit/math/mix/fun/beta_test.cpp \
test/unit/math/mix/fun/binary_log_loss_test.cpp \
test/unit/math/mix/fun/ceil_test.cpp \
test/unit/math/mix/fun/erf_test.cpp \
test/unit/math/mix/fun/erfc_test.cpp \
test/unit/math/mix/fun/exp2_test.cpp \
test/unit/math/mix/fun/expm1_test.cpp \
test/unit/math/mix/fun/falling_factorial_test.cpp \
test/unit/math/mix/fun/floor_test.cpp

Side Effects

One thing I wanted some guidance on, how should we doc the var<matrix> functions when the base version already has documentation? Should we just add a little doc about how this is the function specialized for var matrix types and to see the docs for <function> for further details?

Release notes

Adds var<Matrix> functions for bessel first and second kind, beta, binary log loss, ceil, erf, erfc, exp2, expm1, falling_factorial, floor

Checklist

Math issue How to add static matrix? #1805
Copyright holder: Steve Bronder

The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
- unit tests pass (to run, use: ./runTests.py test/unit)
- header checks pass, (make test-headers)
- dependencies checks pass, (make test-math-dependencies)
- docs build, (make doxygen)
- code passes the built in C++ standards checks (make cpplint)
the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested

…4.1 (tags/RELEASE_600/final)

…matrix

andrjohns · 2021-04-21T00:42:30Z

I can take the review for this one, if nobody else has started yet. Did you have any more changes to add Steve?

SteveBronder · 2021-04-21T01:02:10Z

Much appreciated! No more changes!

stan-buildbot · 2021-04-21T17:53:03Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.39	3.49	0.97	-2.94% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-0.91% slower
eight_schools/eight_schools.stan	0.11	0.11	0.98	-1.85% slower
gp_regr/gp_regr.stan	0.16	0.16	1.02	1.71% faster
irt_2pl/irt_2pl.stan	6.03	6.06	0.99	-0.52% slower
performance.compilation	91.92	89.49	1.03	2.65% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.64	8.61	1.0	0.36% faster
pkpd/one_comp_mm_elim_abs.stan	29.52	30.13	0.98	-2.07% slower
sir/sir.stan	122.24	123.57	0.99	-1.09% slower
gp_regr/gen_gp_data.stan	0.04	0.04	0.99	-1.22% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.05	3.0	1.01	1.39% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.4	0.38	1.05	4.98% faster
arK/arK.stan	1.91	1.91	1.0	-0.28% slower
arma/arma.stan	0.72	0.67	1.06	5.88% faster
garch/garch.stan	0.57	0.57	1.0	0.16% faster
Mean result: 1.00479376746

Jenkins Console Log
Blue Ocean
Commit hash: c73b899

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

andrjohns

Looking good! Just some minor queries.

My big question would be these specialisations only get called if none of the varmat inputs are nested. What happens if a user calls beta(double, std::vector<varmat>) (if that's possible)?

stan/math/prim/fun/beta.hpp

andrjohns · 2021-04-22T14:16:52Z

stan/math/prim/fun/falling_factorial.hpp

@@ -62,7 +62,7 @@ namespace math {
 */
 template <typename T, require_arithmetic_t<T>* = nullptr>
 inline return_type_t<T> falling_factorial(const T& x, int n) {
-  static const char* function = "falling_factorial";
+  constexpr const char* function = "falling_factorial";


Should probably save this for a separate PR

Yeah I can, though it's a teeny change that shouldn't effect much if you don't mind leaving it. In general we should sweep through the math library looking for static const char* function = and make them constexpr

I think probably best to pull it out for now, just so that all functions are consistent. But I'm not super tied to it, so feel free to ignore

Let's leave it for now and I'll open up another PR this week just making a bunch of these constexpr

stan/math/rev/fun/bessel_first_kind.hpp

stan/math/rev/fun/bessel_second_kind.hpp

andrjohns · 2021-04-22T23:51:25Z

stan/math/rev/fun/binary_log_loss.hpp

+  /*
+  if (y == 0) {
+    return make_callback_var(-log1p(-y_hat.val()), [y_hat](auto& vi) mutable {
+      y_hat.adj().array() += vi.adj().array() / (1.0 - y_hat.val().array());
+    });
+  } else {
+    return make_callback_var(-std::log(y_hat.val()), [y_hat](auto& vi) mutable {
+      y_hat.adj().array() -= vi.adj().array() / y_hat.val().array();
+    });
+  }
+  */
+}
+


Should be removed?

andrjohns · 2021-04-23T00:00:02Z

stan/math/rev/fun/exp2.hpp

+template <typename T, require_eigen_t<T>* = nullptr>
+inline auto exp2(const var_value<T>& a) {
+  return make_callback_var(
+      a.val().unaryExpr([](auto&& x) { return std::exp2(x); }),


Can probably just use the Stan Math exp2 here since that's vectorised as well

andrjohns · 2021-04-23T00:01:08Z

stan/math/rev/fun/expm1.hpp

@@ -41,6 +41,13 @@ inline var expm1(const var& a) {
  });
 }

+template <typename T, require_matrix_t<T>* = nullptr>
+inline auto expm1(const var_value<T>& a) {
+  return make_callback_var(expm1(a.val()).eval(), [a](auto& vi) mutable {


Why the eval here when the others don't have it?

stan/math/rev/fun/falling_factorial.hpp

andrjohns · 2021-04-23T00:03:16Z

stan/math/rev/fun/falling_factorial.hpp

+}
+
+template <typename T, typename StdVec, require_eigen_t<T>* = nullptr,
+          require_vector_like_vt<std::is_integral, StdVec>* = nullptr>


I noticed the other functions use std::vector<int> rather the require_, any reason for the preference here?

Honestly idt so, but this is all cleaned up with your suggestions above

…the unary functions

…4.1 (tags/RELEASE_600/final)

SteveBronder · 2021-04-23T19:59:21Z

Much appreciated for the review! I'll ping you when the tests pass

stan-buildbot · 2021-04-24T01:03:19Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.37	3.07	1.1	8.8% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-1.15% slower
eight_schools/eight_schools.stan	0.11	0.12	0.96	-3.77% slower
gp_regr/gp_regr.stan	0.16	0.17	0.91	-9.62% slower
irt_2pl/irt_2pl.stan	6.04	6.07	0.99	-0.59% slower
performance.compilation	92.08	89.49	1.03	2.82% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.89	8.72	1.02	1.89% faster
pkpd/one_comp_mm_elim_abs.stan	29.29	29.26	1.0	0.09% faster
sir/sir.stan	123.23	123.9	0.99	-0.55% slower
gp_regr/gen_gp_data.stan	0.04	0.04	0.92	-8.24% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.07	3.01	1.02	1.97% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.39	1.01	1.11% faster
arK/arK.stan	1.89	1.87	1.01	1.01% faster
arma/arma.stan	0.72	0.75	0.95	-5.21% slower
garch/garch.stan	0.57	0.57	1.0	-0.0% slower
Mean result: 0.994316905174

Jenkins Console Log
Blue Ocean
Commit hash: 6f67a7f

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

SteveBronder · 2021-04-27T15:25:55Z

@andrjohns

SteveBronder · 2021-05-04T17:23:07Z

Pinging @andrjohns

andrjohns

Ack, sorry I missed the first ping! This all looks good, one minor comment around the the constexpr char, but happy to leave it as well

SteveBronder · 2021-05-05T03:02:36Z

Thanks!

My big question would be these specialisations only get called if none of the varmat inputs are nested. What happens if a user calls beta(double, std::vector) (if that's possible)?

Sorry I totally missed this. beta(double, std::vector<varmat>) should call the std::vector<Container> overload. Looking at my code and the tests I think we cover that case.

SteveBronder and others added 6 commits April 7, 2021 17:58

adds simple vectorized functions for var<matrix>

4c6fb92

Merge commit 'b542337b699f6a2377d588ea37d06a0a4927929b' into HEAD

a5c0508

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

fe8e2e3

…4.1 (tags/RELEASE_600/final)

fix expm1 return type

a7958c7

Merge remote-tracking branch 'origin/develop' into feature/unary-var-…

6c16889

…matrix

include require for exp2 for eigen var types

c73b899

andrjohns requested changes Apr 23, 2021

View reviewed changes

SteveBronder and others added 3 commits April 23, 2021 15:58

combine definitions for vector of ints and single int for several of …

e917df0

…the unary functions

Merge commit 'f1905f68276b9cc578bb1c7ab3f8e42266e0074a' into HEAD

e62c148

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

6f67a7f

…4.1 (tags/RELEASE_600/final)

andrjohns approved these changes May 4, 2021

View reviewed changes

SteveBronder merged commit 1dee1dc into develop May 5, 2021

SteveBronder mentioned this pull request Jun 9, 2021

adds testing suite for nested binary var matrix functions #2502

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds simple vectorized functions for var<matrix> #2461

Adds simple vectorized functions for var<matrix> #2461

SteveBronder commented Apr 7, 2021

andrjohns commented Apr 21, 2021

SteveBronder commented Apr 21, 2021

stan-buildbot commented Apr 21, 2021

andrjohns left a comment

andrjohns Apr 22, 2021

SteveBronder Apr 23, 2021

andrjohns May 4, 2021

SteveBronder May 5, 2021

andrjohns Apr 22, 2021

SteveBronder Apr 23, 2021

andrjohns Apr 23, 2021

andrjohns Apr 23, 2021

andrjohns Apr 23, 2021

SteveBronder Apr 23, 2021

SteveBronder commented Apr 23, 2021

stan-buildbot commented Apr 24, 2021

SteveBronder commented Apr 27, 2021

SteveBronder commented May 4, 2021

andrjohns left a comment

SteveBronder commented May 5, 2021

Adds simple vectorized functions for var<matrix> #2461

Adds simple vectorized functions for var<matrix> #2461

Conversation

SteveBronder commented Apr 7, 2021

Summary

Tests

Side Effects

Release notes

Checklist

andrjohns commented Apr 21, 2021

SteveBronder commented Apr 21, 2021

stan-buildbot commented Apr 21, 2021

andrjohns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveBronder commented Apr 23, 2021

stan-buildbot commented Apr 24, 2021

SteveBronder commented Apr 27, 2021

SteveBronder commented May 4, 2021

andrjohns left a comment

Choose a reason for hiding this comment

SteveBronder commented May 5, 2021