adds unary var matrix functions #2362

SteveBronder · 2021-02-10T00:18:43Z

Summary

Adds var<Matrix> specializations for digamma, distance, Phi, Phi_approx, inv_Phi, sqrt, tail, and tgamma.

Tests

var matrix tests added for each function and can be run with

./runTests.py test/unit/math/mix/fun/digamma_test.cpp \
test/unit/math/mix/fun/distance_test.cpp \
test/unit/math/mix/fun/inv_Phi_test.cpp \
test/unit/math/mix/fun/Phi_test.cpp \
test/unit/math/mix/fun/Phi_approx_test.cpp \
test/unit/math/mix/fun/sqrt_test.cpp \
test/unit/math/mix/fun/tail_test.cpp \
test/unit/math/mix/fun/tgamma_test.cpp

Side Effects

Nope!

Release notes

var<Matrix> overloads for digamma, distance, Phi, inv_Phi, Phi_approx, sqrt, tail, and tgamma

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)

…unc1

stan-buildbot · 2021-02-10T09:32:51Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.46	3.47	1.0	-0.34% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	1.02	2.22% faster
eight_schools/eight_schools.stan	0.11	0.11	1.01	0.87% faster
gp_regr/gp_regr.stan	0.15	0.15	1.0	0.03% faster
irt_2pl/irt_2pl.stan	5.29	5.21	1.02	1.49% faster
performance.compilation	89.44	88.51	1.01	1.04% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	7.82	7.86	0.99	-0.54% slower
pkpd/one_comp_mm_elim_abs.stan	28.69	28.55	1.0	0.49% faster
sir/sir.stan	127.76	128.21	1.0	-0.36% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	0.43% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.02	3.01	1.0	0.18% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.39	0.98	-1.85% slower
arK/arK.stan	2.48	2.5	0.99	-0.8% slower
arma/arma.stan	0.72	0.76	0.95	-4.91% slower
garch/garch.stan	0.65	0.73	0.89	-12.69% slower
Mean result: 0.991388117501

Jenkins Console Log
Blue Ocean
Commit hash: e525689

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

rok-cesnovar

This looks good to me. I have one minor question.

My first time reviewing varmat stuff so hopefully I didnt miss anything.

rok-cesnovar · 2021-02-10T12:01:19Z

stan/math/rev/fun/Phi_approx.hpp

+template <typename T, require_var_matrix_t<T>* = nullptr>
+inline auto Phi_approx(const T& a) {
+  arena_t<value_type_t<T>> f(a.rows(), a.cols());
+  arena_t<value_type_t<T>> da(a.rows(), a.cols());


Does it make sense to compute this in the forward pass? I guess we would compute av_squared twice if we compute this in the backward pass. Is that the reason?

yeah that's the general jist. we can compute adjoint while calculating the forward pass so it's just nice to alloc everything at once, throw it in one big loop, and then have an easy peasy reverse pass.

rok-cesnovar

The varmat rev functions are missing doxygen. Other than that I think this looks good.

rok-cesnovar · 2021-02-10T13:55:52Z

stan/math/rev/fun/Phi.hpp

+  });
+}
+
+template <typename T, require_var_matrix_t<T>* = nullptr>


We probably need doxygen here?

…unc1

bbbales2

I added doxygen and simplified a couple tests. This looks good to me. @SteveBronder check my commits and merge if the tests pass.

stan-buildbot · 2021-02-24T09:56:17Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.47	3.61	0.96	-4.06% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	1.0	-0.44% slower
eight_schools/eight_schools.stan	0.11	0.12	0.96	-4.66% slower
gp_regr/gp_regr.stan	0.15	0.15	1.01	0.8% faster
irt_2pl/irt_2pl.stan	5.22	5.26	0.99	-0.81% slower
performance.compilation	90.57	88.3	1.03	2.51% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	7.81	7.83	1.0	-0.34% slower
pkpd/one_comp_mm_elim_abs.stan	29.87	27.91	1.07	6.55% faster
sir/sir.stan	132.01	135.76	0.97	-2.83% slower
gp_regr/gen_gp_data.stan	0.04	0.04	0.99	-1.09% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.1	3.02	1.03	2.56% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.4	0.98	-2.55% slower
arK/arK.stan	2.56	2.52	1.01	1.43% faster
arma/arma.stan	0.68	0.68	1.0	-0.2% slower
garch/garch.stan	0.65	0.74	0.88	-13.59% slower
Mean result: 0.990699388087

Jenkins Console Log
Blue Ocean
Commit hash: f34fc2d

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

rok-cesnovar

Good!

SteveBronder and others added 5 commits February 9, 2021 18:59

adds unary var matrix functions

8262c78

adds sqrt test

400a7b1

add inv_Phi

48671e5

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

a6f95a2

…4.1 (tags/RELEASE_600/final)

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

e525689

…unc1

rok-cesnovar reviewed Feb 10, 2021

View reviewed changes

rok-cesnovar requested changes Feb 10, 2021

View reviewed changes

bbbales2 added 3 commits February 23, 2021 16:27

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

e14ad85

…unc1

Added Phi comment (Issue #1805)

bfe60c0

Updated doxygen a bit (Issue #1805)

d51a225

bbbales2 previously approved these changes Feb 23, 2021

View reviewed changes

Added missing comma (Issue #1805)

f34fc2d

bbbales2 dismissed their stale review via f34fc2d February 24, 2021 00:31

rok-cesnovar approved these changes Feb 24, 2021

View reviewed changes

rok-cesnovar merged commit 10519ce into develop Feb 24, 2021

rok-cesnovar deleted the feature/misc-var-func1 branch February 24, 2021 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adds unary var matrix functions #2362

adds unary var matrix functions #2362

SteveBronder commented Feb 10, 2021

stan-buildbot commented Feb 10, 2021

rok-cesnovar left a comment

rok-cesnovar Feb 10, 2021

SteveBronder Feb 10, 2021

rok-cesnovar Feb 10, 2021

rok-cesnovar left a comment

rok-cesnovar Feb 10, 2021

bbbales2 left a comment

stan-buildbot commented Feb 24, 2021

rok-cesnovar left a comment

adds unary var matrix functions #2362

adds unary var matrix functions #2362

Conversation

SteveBronder commented Feb 10, 2021

Summary

Tests

Side Effects

Release notes

Checklist

stan-buildbot commented Feb 10, 2021

rok-cesnovar left a comment

Choose a reason for hiding this comment

rok-cesnovar Feb 10, 2021

Choose a reason for hiding this comment

SteveBronder Feb 10, 2021

Choose a reason for hiding this comment

rok-cesnovar Feb 10, 2021

Choose a reason for hiding this comment

rok-cesnovar left a comment

Choose a reason for hiding this comment

rok-cesnovar Feb 10, 2021

Choose a reason for hiding this comment

bbbales2 left a comment

Choose a reason for hiding this comment

stan-buildbot commented Feb 24, 2021

rok-cesnovar left a comment

Choose a reason for hiding this comment