operator- for var<matrix> #2196

SteveBronder · 2020-11-16T06:51:02Z

Summary

Adds operator- for var<matrix> types

Tests

Adds the same tests for operator+ for var<matrix>

Side Effects

Nope!

Release notes

Adds operator- for var<matrix> types

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

…rator-minus

…4.1 (tags/RELEASE_600/final)

stan/math/rev/core/operator_subtraction.hpp

SteveBronder · 2020-11-16T19:57:33Z

@t4c1 can you take a look at beta proportion? I'm not sure if it's just some machines but in the add PR and this PR the beta proportion test has failed intermittently

[----------] 6 tests from ProbDistributionsBetaProportion
[ RUN      ] ProbDistributionsBetaProportion.error_checking
[       OK ] ProbDistributionsBetaProportion.error_checking (19 ms)
[ RUN      ] ProbDistributionsBetaProportion.opencl_matches_cpu_small
[       OK ] ProbDistributionsBetaProportion.opencl_matches_cpu_small (63 ms)
[ RUN      ] ProbDistributionsBetaProportion.opencl_broadcast_y
[       OK ] ProbDistributionsBetaProportion.opencl_broadcast_y (89 ms)
[ RUN      ] ProbDistributionsBetaProportion.opencl_broadcast_mu
test/unit/math/opencl/rev/beta_proportion_lpdf_test --gtest_output="xml:test/unit/math/opencl/rev/beta_proportion_lpdf_test.xml" failed
exit now (11/16/20 14:00:30 EST)

rok-cesnovar · 2020-11-16T20:05:15Z

I think you just need to merge recent develop. This was fixed by #2187

…rator-minus

SteveBronder · 2020-11-16T20:23:42Z

Ah word! Just merged to develop

stan-buildbot · 2020-11-17T02:42:46Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.02	3.63	0.83	-20.45% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.98	-1.54% slower
eight_schools/eight_schools.stan	0.12	0.11	1.03	2.45% faster
gp_regr/gp_regr.stan	0.17	0.17	1.01	1.46% faster
irt_2pl/irt_2pl.stan	5.71	5.87	0.97	-2.81% slower
performance.compilation	86.64	85.57	1.01	1.23% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.48	8.47	1.0	0.09% faster
pkpd/one_comp_mm_elim_abs.stan	31.27	28.29	1.11	9.54% faster
sir/sir.stan	133.15	130.51	1.02	1.98% faster
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	0.26% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	2.99	2.96	1.01	0.89% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.37	0.39	0.96	-4.06% slower
arK/arK.stan	1.77	1.78	0.99	-0.61% slower
arma/arma.stan	0.61	0.59	1.04	3.73% faster
garch/garch.stan	0.74	0.56	1.34	25.12% faster
Mean result: 1.02049338032

Jenkins Console Log
Blue Ocean
Commit hash: 0b22db8

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

stan-buildbot · 2020-11-18T01:02:55Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.01	3.6	0.84	-19.59% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.93	-7.26% slower
eight_schools/eight_schools.stan	0.12	0.12	0.99	-0.86% slower
gp_regr/gp_regr.stan	0.17	0.16	1.01	1.21% faster
irt_2pl/irt_2pl.stan	5.66	5.85	0.97	-3.36% slower
performance.compilation	87.72	85.42	1.03	2.62% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.48	8.48	1.0	0.08% faster
pkpd/one_comp_mm_elim_abs.stan	30.78	29.32	1.05	4.71% faster
sir/sir.stan	131.07	133.7	0.98	-2.0% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	0.14% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.01	2.95	1.02	1.97% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.37	1.05	4.96% faster
arK/arK.stan	1.77	1.77	1.0	0.44% faster
arma/arma.stan	0.6	0.61	0.99	-1.21% slower
garch/garch.stan	0.75	0.55	1.36	26.33% faster
Mean result: 1.01472132003

Jenkins Console Log
Blue Ocean
Commit hash: b70ddae

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

…rator-minus

stan-buildbot · 2020-11-25T03:48:21Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.57	3.67	0.97	-2.57% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	1.02	2.31% faster
eight_schools/eight_schools.stan	0.12	0.11	1.0	0.29% faster
gp_regr/gp_regr.stan	0.17	0.17	1.0	-0.01% slower
irt_2pl/irt_2pl.stan	5.63	5.8	0.97	-3.16% slower
performance.compilation	88.23	85.49	1.03	3.11% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.43	8.49	0.99	-0.71% slower
pkpd/one_comp_mm_elim_abs.stan	30.14	31.4	0.96	-4.17% slower
sir/sir.stan	132.59	127.42	1.04	3.9% faster
gp_regr/gen_gp_data.stan	0.04	0.04	0.99	-1.18% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.0	2.95	1.02	1.61% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.4	0.37	1.07	6.89% faster
arK/arK.stan	2.5	2.47	1.01	1.42% faster
arma/arma.stan	0.6	0.6	1.0	0.37% faster
garch/garch.stan	0.76	0.55	1.38	27.71% faster
Mean result: 1.03176363118

Jenkins Console Log
Blue Ocean
Commit hash: 275276e

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 · 2020-11-25T18:46:32Z

Could someone give this a look? Think it is good to go!

bbbales2 · 2020-11-25T19:28:48Z

Yeah I'll do it later

bbbales2

Looks good. I left a couple pedantic comments in the tests you can take or leave.

Do you mind looking at this first before we merge this: https://github.com/stan-dev/math/pull/2213/files#r530656277

I think that's related to whatever we do here (since that's operator+).

bbbales2 · 2020-11-26T01:00:48Z

test/unit/math/mix/core/operator_subtraction_test.cpp

+  stan::test::expect_ad_matvar(tols, f, scalar_a, row_vector_rv1);
+  stan::test::expect_ad_matvar(tols, f, row_vector_rv1, scalar_a);
+  stan::test::expect_ad_matvar(tols, f, row_vector_rv1, vector_v1);
+  stan::test::expect_ad_matvar(tols, f, vector_v1, row_vector_rv1);


These mixed type tests strike me as weird. I kinda want them gone cause they might be accidentally working now cause the linear sizes of the inputs are the same (and in reality I don't think we want to do the 1d vector ambiguity thing cause we don't do it at the language).

I guess this is only checking that varmat behavior matches matvar, which is checking against prim, which is hopefully doing the right thing.

Edit: swapped varmat and matvar

Yeah, I mean I think it's okay to do here in math. As long as their is not a signature for it the language won't access it.

bbbales2 · 2020-11-26T01:01:27Z

test/unit/math/mix/core/operator_subtraction_test.cpp

+  stan::test::expect_ad_matvar(tols, f, scalar_a, matrix_m);
+  stan::test::expect_ad_matvar(tols, f, matrix_m, scalar_a);
+  stan::test::expect_ad_matvar(tols, f, matrix_m, vector_v);
+  stan::test::expect_ad_matvar(tols, f, rowvector_rv, matrix_m);


These should definitely just be throwing errors. I guess it's good to have them so the error checks between mat<var> and var<mat> are consistent. Probably helpful to put them off to the side and say "errors".

I put a space there to break them out

stan-buildbot · 2020-11-26T10:16:28Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.65	3.59	1.02	1.82% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	1.0	-0.2% slower
eight_schools/eight_schools.stan	0.12	0.12	0.99	-0.94% slower
gp_regr/gp_regr.stan	0.17	0.17	1.01	0.7% faster
irt_2pl/irt_2pl.stan	5.68	5.88	0.97	-3.47% slower
performance.compilation	86.45	85.7	1.01	0.87% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.45	8.45	1.0	-0.08% slower
pkpd/one_comp_mm_elim_abs.stan	29.15	28.68	1.02	1.61% faster
sir/sir.stan	135.0	131.89	1.02	2.3% faster
gp_regr/gen_gp_data.stan	0.04	0.05	0.98	-1.76% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	2.93	2.95	1.0	-0.48% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.38	0.37	1.02	2.06% faster
arK/arK.stan	2.49	2.47	1.01	0.69% faster
arma/arma.stan	0.6	0.61	0.98	-1.69% slower
garch/garch.stan	0.76	0.55	1.38	27.73% faster
Mean result: 1.02678775532

Jenkins Console Log
Blue Ocean
Commit hash: a3b1f4f

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

bbbales2 · 2020-11-27T17:00:26Z

@SteveBronder this looks good. I'm gonna go ahead and merge. If it ends up there's a problem with operator+ over here #2213 (review) then we'll just need to fix operator- as well.

bbbales2 · 2020-11-27T17:01:31Z

@t4c1 your review is still here. Have a look and merge when you're happy with this.

SteveBronder added 3 commits November 13, 2020 17:20

adds subtraction for var<mat>

3804a41

Merge remote-tracking branch 'origin/develop' into feature/varmat-ope…

1d7cd2e

…rator-minus

fix headers for var subtract

92dd2f3

SteveBronder changed the title ~~Feature/varmat operator minus~~ operator- for var<matrix> Nov 16, 2020

SteveBronder and others added 2 commits November 16, 2020 01:53

fix typo in comment for test

f01d93e

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

37cf4a5

…4.1 (tags/RELEASE_600/final)

t4c1 requested changes Nov 16, 2020

View reviewed changes

fix subtract so expressions are only evaluated once

fc59844

Merge remote-tracking branch 'origin/develop' into feature/varmat-ope…

0b22db8

…rator-minus

check just the return for nan in scalar var subtraction and fix docs

b70ddae

SteveBronder added 3 commits November 19, 2020 14:28

Merge remote-tracking branch 'origin/develop' into feature/varmat-ope…

ae6bc26

…rator-minus

Merge remote-tracking branch 'origin/develop' into feature/varmat-ope…

262da03

…rator-minus

Merge remote-tracking branch 'origin/develop' into feature/varmat-ope…

275276e

…rator-minus

bbbales2 requested changes Nov 26, 2020

View reviewed changes

space out the tests that fail

a3b1f4f

bbbales2 approved these changes Nov 27, 2020

View reviewed changes

bbbales2 mentioned this pull request Nov 29, 2020

Make functions with custom autodiff var<mat> friendly #2101

Open

78 tasks

t4c1 approved these changes Nov 30, 2020

View reviewed changes

t4c1 merged commit 0c1ef70 into develop Nov 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

operator- for var<matrix> #2196

operator- for var<matrix> #2196

SteveBronder commented Nov 16, 2020

SteveBronder commented Nov 16, 2020

rok-cesnovar commented Nov 16, 2020

SteveBronder commented Nov 16, 2020

stan-buildbot commented Nov 17, 2020

stan-buildbot commented Nov 18, 2020

stan-buildbot commented Nov 25, 2020

SteveBronder commented Nov 25, 2020

bbbales2 commented Nov 25, 2020

bbbales2 left a comment

bbbales2 Nov 26, 2020

bbbales2 Nov 26, 2020 •

edited

Loading

SteveBronder Nov 26, 2020

bbbales2 Nov 26, 2020

SteveBronder Nov 26, 2020

stan-buildbot commented Nov 26, 2020

bbbales2 commented Nov 27, 2020

bbbales2 commented Nov 27, 2020

operator- for var<matrix> #2196

operator- for var<matrix> #2196

Conversation

SteveBronder commented Nov 16, 2020

Summary

Tests

Side Effects

Release notes

Checklist

SteveBronder commented Nov 16, 2020

rok-cesnovar commented Nov 16, 2020

SteveBronder commented Nov 16, 2020

stan-buildbot commented Nov 17, 2020

stan-buildbot commented Nov 18, 2020

stan-buildbot commented Nov 25, 2020

SteveBronder commented Nov 25, 2020

bbbales2 commented Nov 25, 2020

bbbales2 left a comment

Choose a reason for hiding this comment

bbbales2 Nov 26, 2020

Choose a reason for hiding this comment

bbbales2 Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

SteveBronder Nov 26, 2020

Choose a reason for hiding this comment

bbbales2 Nov 26, 2020

Choose a reason for hiding this comment

SteveBronder Nov 26, 2020

Choose a reason for hiding this comment

stan-buildbot commented Nov 26, 2020

bbbales2 commented Nov 27, 2020

bbbales2 commented Nov 27, 2020

bbbales2 Nov 26, 2020 •

edited

Loading