Add singular_values, svd_U, and svd_V #2286

Dashadower · 2021-01-02T13:23:22Z

Summary

This implements singular_values, svd_U, and svd_V, with its respective gradients based on Eigen's JacobiSVD

Tests

test/unit/math/rev/fun/singularvalues_test.cpp
test/unit/math/prim/fun/svd_U_test.cpp
test/unit/math/prim/fun/svd_V_test.cpp
test/unit/math/mix/fun/svd_U_test.cpp
test/unit/math/mix/fun/svd_V_test.cpp

Side Effects

svd_U and svd_V may have reversed column-wise signs from other SVD library's results.

Release notes

Implement svd_U and svd_V, add gradients to singular_values.

Checklist

Math issue SVD functions #2192
Copyright holder: Hyunji Moon, Ben Bales, Shinyoung Kim

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

…and added mixed mode tests (Issue stan-dev#2101)

…svd_V} functions to return thinned matrices instead of full

…4.1 (tags/RELEASE_600/final)

stan-buildbot · 2021-01-03T00:33:48Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.37	3.43	0.98	-1.63% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-0.58% slower
eight_schools/eight_schools.stan	0.11	0.11	0.99	-1.05% slower
gp_regr/gp_regr.stan	0.15	0.15	0.98	-1.71% slower
irt_2pl/irt_2pl.stan	5.24	5.24	1.0	-0.02% slower
performance.compilation	93.22	90.09	1.03	3.36% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.76	8.67	1.01	1.0% faster
pkpd/one_comp_mm_elim_abs.stan	29.5	28.46	1.04	3.51% faster
sir/sir.stan	136.78	138.22	0.99	-1.06% slower
gp_regr/gen_gp_data.stan	0.04	0.04	0.99	-1.32% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.05	3.05	1.0	-0.2% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.38	0.39	0.97	-2.74% slower
arK/arK.stan	2.52	2.52	1.0	-0.14% slower
arma/arma.stan	0.6	0.61	0.98	-1.64% slower
garch/garch.stan	0.67	0.67	1.0	-0.44% slower
Mean result: 0.997190897201

Jenkins Console Log
Blue Ocean
Commit hash: 8d41dd4

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

The singular values prim tests need more in there. Just use the same matrices you test w/ svd_U and svd_V. We apparently were lazy when we implemented this (https://github.com/stan-dev/math/blob/develop/test/unit/math/prim/fun/singular_values_test.cpp).

Also add a 2x3 matrix to the mix singular_values test.

There's another set of changes to make this support the new varmat matrix type (https://github.com/stan-dev/design-docs/blob/master/designs/0005-static-matrices.md).

I think all the comments I left here are pretty easy to straighten out (we'll wait for some feedback on the matrices). Ping me after you go through them, and I'll add instructions on doing the varmat stuff. It's another bunch of small changes. I've apparently left 14 comments here which is enuff stuff for one review tho :D.

stan/math/rev/fun/singular_values.hpp

stan/math/prim/fun/svd_U.hpp

stan/math/rev/fun/singular_values.hpp

stan/math/rev/fun/svd_U.hpp

bbbales2 · 2021-01-03T18:13:29Z

stan/math/prim/fun/singular_values.hpp

@@ -17,7 +17,8 @@ namespace math {
 * @param m Specified matrix.
 * @return Singular values of the matrix.
 */
-template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr>
+template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr,
+          require_not_vt_var<EigMat>* = nullptr>


It looks like this returns matrices of size zero instead of throwing. We'll need to make this consistent across all the functions but let's wait for a response to the question in prim/fun/svd_U.hpp first.

Just make the prim version throw I think. That'll be simplest for now. We can roll it back. Later.

So instead of returning a 0x0, do: check_nonzero_size("singular_values", "m", m);

stan/math/rev/fun/svd_U.hpp

stan/math/rev/fun/svd_V.hpp

stan-buildbot · 2021-01-05T00:50:22Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.48	3.4	1.02	2.06% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-1.27% slower
eight_schools/eight_schools.stan	0.11	0.12	0.9	-10.64% slower
gp_regr/gp_regr.stan	0.15	0.15	1.0	-0.01% slower
irt_2pl/irt_2pl.stan	5.2	5.15	1.01	0.94% faster
performance.compilation	92.45	90.14	1.03	2.5% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.73	8.65	1.01	0.82% faster
pkpd/one_comp_mm_elim_abs.stan	29.93	31.5	0.95	-5.24% slower
sir/sir.stan	136.07	135.09	1.01	0.71% faster
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	-0.04% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.05	3.05	1.0	-0.18% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.37	0.37	1.01	1.29% faster
arK/arK.stan	2.51	2.51	1.0	0.02% faster
arma/arma.stan	0.6	0.62	0.97	-2.93% slower
garch/garch.stan	0.67	0.67	1.0	0.06% faster
Mean result: 0.993077964502

Jenkins Console Log
Blue Ocean
Commit hash: 8d41dd4

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

…re/svd_review

Dashadower · 2021-01-05T14:40:41Z

@bbbales2 Changed as you requested. There already seems to be a 2x3 matrix test in test/unit/math/mix/fun/singular_values_test.cpp.

…4.1 (tags/RELEASE_600/final)

stan-buildbot · 2021-01-05T21:52:38Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.38	3.42	0.99	-1.12% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-1.46% slower
eight_schools/eight_schools.stan	0.11	0.11	1.02	1.75% faster
gp_regr/gp_regr.stan	0.15	0.16	0.97	-2.98% slower
irt_2pl/irt_2pl.stan	5.19	5.17	1.0	0.37% faster
performance.compilation	91.26	90.25	1.01	1.11% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.64	8.77	0.99	-1.47% slower
pkpd/one_comp_mm_elim_abs.stan	29.15	29.09	1.0	0.18% faster
sir/sir.stan	136.72	138.92	0.98	-1.61% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.01	0.81% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.05	3.05	1.0	0.03% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.39	1.0	0.33% faster
arK/arK.stan	2.55	2.52	1.01	1.25% faster
arma/arma.stan	0.61	0.6	1.01	0.74% faster
garch/garch.stan	0.67	0.68	0.99	-1.46% slower
Mean result: 0.997829736444

Jenkins Console Log
Blue Ocean
Commit hash: f47682b

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

Alright, the varmat conversion. I left directions in the comments. Lemme know what I forgot (cause I probably did and the error messages will inevitably be confusing)

bbbales2 · 2021-01-06T19:11:49Z

stan/math/prim/fun/singular_values.hpp

@@ -17,7 +17,8 @@ namespace math {
 * @param m Specified matrix.
 * @return Singular values of the matrix.
 */
-template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr>
+template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr,
+          require_not_vt_var<EigMat>* = nullptr>


Just make the prim version throw I think. That'll be simplest for now. We can roll it back. Later.

So instead of returning a 0x0, do: check_nonzero_size("singular_values", "m", m);

bbbales2 · 2021-01-06T19:15:31Z

stan/math/prim/fun/singular_values.hpp

@@ -17,7 +17,8 @@ namespace math {
 * @param m Specified matrix.
 * @return Singular values of the matrix.
 */
-template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr>
+template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr,
+          require_not_vt_var<EigMat>* = nullptr>


Alright so we'll do the varmat conversion here first. Along with Eigen::Matrix<var, R, C>, we just added a new autodiff type to Stan (this one). We don't have developer docs up so lemme walk you through how to convert this.

First change require_not_vt_var<EigMat> to require_not_st_var<EigMat>.

The first says "Require the value type of the template parameter to not be a var". Varmats are var_value<Eigen::Matrix<T, R, C>> types, and the value type of a var_value<Eigen::Matrix<T, R, C>> is Eigen::Matrix<T, R, C>. The scalar type though is var.

So for this to work with varmat we want to say "Require the scalar type of the template parameter to not be a var"

bbbales2 · 2021-01-06T19:16:53Z

stan/math/rev/fun/singular_values.hpp

+ * @param m MxN input matrix
+ * @return Singular values of matrix
+ */
+template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr,


Change: require_eigen_matrix_dynamic_t<EigMat> to require_rev_matrix_t<EigMat>

That means accept Eigen::Matrix<var, R, C> and var_value<Eigen::Matrix<double, R, C>> types, not just the first.

Delete require_vt_var<EigMat>. Not necessary.

bbbales2 · 2021-01-06T19:19:05Z

stan/math/rev/fun/singular_values.hpp

+template <typename EigMat, require_eigen_matrix_dynamic_t<EigMat>* = nullptr,
+          require_vt_var<EigMat>* = nullptr>
+inline auto singular_values(const EigMat& m) {
+  using ret_type = promote_scalar_t<var, Eigen::VectorXd>;


using ret_type = return_var_matrix_t<Eigen::VectorXd, EigMat>;

This computes the autodiff return type for a non-autodiff type Eigen::VectorXd given the input argument is EigMat. If this is an Eigen::Matrix<var, R, C> type, the output will similarly be an Eigen matrix of vars (call this a matvar for short). If the input is a var_value<Eigen::Matrix<double, R, C>>, the output will be a var_value<Eigen::VectorXd> (call this a varmat for short). Both are valid vector autodiff types in Stan.

I think with that singular_values should work with varmats. Now we gotta add tests.

bbbales2 · 2021-01-06T19:20:52Z

test/unit/math/mix/fun/svd_U_test.cpp

+  tols.hessian_fvar_hessian_ = 1e-2;
+
+  Eigen::MatrixXd m00(0, 0);
+  stan::test::expect_ad(f, m00);


To do varmat tests, for every stan::test::expect_ad(f, ...) do a stan::test::expect_ad_matvar(f, ...).

And that's that! This should work for singular_values, svd_U, and svd_V. Lemme know if you have problems.

…re/svd_review

…ances in tests (Issue stan-dev#2101)

…4.1 (tags/RELEASE_600/final)

bbbales2 · 2021-01-10T16:27:06Z

@Dashadower I made a few edits, check em' here. If you're good with those, I'll merge this when the tests pass.

stan-buildbot · 2021-01-10T23:58:23Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.41	3.4	1.0	0.13% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-0.71% slower
eight_schools/eight_schools.stan	0.11	0.12	0.9	-10.76% slower
gp_regr/gp_regr.stan	0.15	0.15	1.0	-0.34% slower
irt_2pl/irt_2pl.stan	5.2	5.2	1.0	0.07% faster
performance.compilation	92.74	90.19	1.03	2.75% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.62	8.64	1.0	-0.19% slower
pkpd/one_comp_mm_elim_abs.stan	28.12	29.01	0.97	-3.15% slower
sir/sir.stan	135.37	137.94	0.98	-1.9% slower
gp_regr/gen_gp_data.stan	0.04	0.04	0.99	-1.29% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.05	3.04	1.0	0.07% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.38	0.38	1.0	0.43% faster
arK/arK.stan	2.55	2.51	1.02	1.65% faster
arma/arma.stan	0.6	0.6	1.0	0.09% faster
garch/garch.stan	0.57	0.56	1.0	0.47% faster
Mean result: 0.9924286126

Jenkins Console Log
Blue Ocean
Commit hash: 784284f

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

hyunjimoon and others added 30 commits October 5, 2020 22:53

implement interp1

63237ea

reverse svd and its test

71fe4b6

clean interp1 and add to function header file

f7b127c

Formatting and such (Issue stan-dev#2192)

77178ab

update reverse mode on deleting D and add to_arena

dc43ce0

Finish singular_values.hpp, verify tests pass

c596c4d

Remove unnecessary code from singular_values.hpp

2ba5173

Use arena_t and remove redundant comments in singular_values.hpp

81b442a

Couple cleanup things (Issue stan-dev#2192)

8a3a324

WIP svd_U function and tests

f674c6c

Change to use matrix_v for matrix U

09e7ecf

Merge remote-tracking branch 'origin/develop' into feature/svd_review

ea7e589

Merging in develop, adding .val_op() and .adj_op()

c097d11

Merge remote-tracking branch 'origin/develop' into feature/svd

034471f

Merge branch 'feature/svd' into feature/svd_review

d258e61

Merge remote-tracking branch 'origin/develop' into hyunji-develop

be43569

Merge remote-tracking branch 'hyunji/develop' into feature/svd

5f121a0

Merge branch 'feature/svd' into feature/svd_review

12485ad

Minor tweaks to svd_U and corresponding test suite

3b33ec8

Renamed singular_values_U to svd_U, add prim version of svd_U, …

156acea

…and added mixed mode tests (Issue stan-dev#2101)

Updated tests again (Issue stan-dev#2101)

7ba6df0

Add svd_V function

aa43a00

Address non-square inputs for svd_U and svd_V

7df7307

Add prim tests for svd_V, svd_U

e1c7dcc

Updated svd V test

7ec29e8

Update tests for prim/fun/{svd_U, svd_V}.hpp. Change rev/fun/{svd_U, …

a7c21ec

…svd_V} functions to return thinned matrices instead of full

Update prim tests

e057457

Update mix mode tests

63defd1

Merge commit 'be49360ca7eb65addfa7001c04699fd6f1a85330' into HEAD

bb67570

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

7873afc

…4.1 (tags/RELEASE_600/final)

bbbales2 self-requested a review January 2, 2021 16:02

Dashadower and others added 4 commits January 3, 2021 01:48

Fix lint warnings

59aefc0

Update doxygen warnings

52889c4

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

c4cb965

…4.1 (tags/RELEASE_600/final)

Update doxygen docs

8d41dd4

bbbales2 requested changes Jan 3, 2021

View reviewed changes

Dashadower added 2 commits January 5, 2021 22:39

Merge branch 'develop' of https://github.com/stan-dev/math into featu…

eb17cfa

…re/svd_review

Update tests and functions for svd

e8713e4

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

f47682b

…4.1 (tags/RELEASE_600/final)

bbbales2 requested changes Jan 6, 2021

View reviewed changes

Dashadower and others added 5 commits January 11, 2021 00:29

Merge branch 'develop' of https://github.com/stan-dev/math into featu…

3b025fc

…re/svd_review

Add vari support and relevant tests

c3835c6

Removed extra comments, rearranged includes, and removed custom toler…

02c5a81

…ances in tests (Issue stan-dev#2101)

Merge commit '88040ea5e8e8bc308ee080a8dcab9d5452f40ecf' into HEAD

091ce12

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

784284f

…4.1 (tags/RELEASE_600/final)

bbbales2 approved these changes Jan 11, 2021

View reviewed changes

bbbales2 merged commit 222e1ad into stan-dev:develop Jan 11, 2021

bbbales2 mentioned this pull request Jan 11, 2021

Signatures for svd_U and svd_V stan-dev/stanc3#790

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add singular_values, svd_U, and svd_V #2286

Add singular_values, svd_U, and svd_V #2286

Dashadower commented Jan 2, 2021 •

edited

Loading

stan-buildbot commented Jan 3, 2021

bbbales2 left a comment

bbbales2 Jan 3, 2021

bbbales2 Jan 6, 2021

stan-buildbot commented Jan 5, 2021

Dashadower commented Jan 5, 2021

stan-buildbot commented Jan 5, 2021

bbbales2 left a comment

bbbales2 Jan 6, 2021

bbbales2 Jan 6, 2021

bbbales2 Jan 6, 2021

bbbales2 Jan 6, 2021

bbbales2 Jan 6, 2021

bbbales2 Jan 6, 2021

bbbales2 commented Jan 10, 2021

stan-buildbot commented Jan 10, 2021

Add singular_values, svd_U, and svd_V #2286

Add singular_values, svd_U, and svd_V #2286

Conversation

Dashadower commented Jan 2, 2021 • edited Loading

Summary

Tests

Side Effects

Release notes

Checklist

stan-buildbot commented Jan 3, 2021

bbbales2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stan-buildbot commented Jan 5, 2021

Dashadower commented Jan 5, 2021

stan-buildbot commented Jan 5, 2021

bbbales2 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

bbbales2 commented Jan 10, 2021

stan-buildbot commented Jan 10, 2021

Dashadower commented Jan 2, 2021 •

edited

Loading