Skip to content
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.

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

Don't throw when computing the inverse of a size zero matrix #1701

Merged
merged 2 commits into from
Feb 13, 2020

Conversation

mcol
Copy link
Contributor

@mcol mcol commented Feb 11, 2020

Summary

This reverts the earlier decision to throw when computing the inverse of a size zero matrix, as discussed in #1689. We need to return an empty matrix ourselves because Eigen asserts when computing the inverse of an empty matrix. Fixes #1463. Fixes #1432.

Tests

Updated accordingly. There were also a couple of errors in those tests concerning the type of exception thrown.

Side Effects

None.

Checklist

@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.84 5.07 0.95 -4.78% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.99 -0.8% slower
eight_schools/eight_schools.stan 0.09 0.09 1.03 3.17% faster
gp_regr/gp_regr.stan 0.22 0.22 1.01 0.9% faster
irt_2pl/irt_2pl.stan 6.06 6.06 1.0 -0.03% slower
performance.compilation 89.76 87.1 1.03 2.96% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.48 7.49 1.0 -0.09% slower
pkpd/one_comp_mm_elim_abs.stan 21.33 21.17 1.01 0.75% faster
sir/sir.stan 89.74 98.18 0.91 -9.4% slower
gp_regr/gen_gp_data.stan 0.05 0.05 0.99 -1.35% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.01 3.0 1.0 0.3% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.34 0.32 1.07 6.89% faster
arK/arK.stan 1.75 1.74 1.0 0.32% faster
arma/arma.stan 0.8 0.79 1.01 0.94% faster
garch/garch.stan 0.59 0.59 1.0 -0.04% slower
Mean result: 1.00100474161

Jenkins Console Log
Blue Ocean
Commit hash: 2f875bd


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

bob-carpenter
bob-carpenter previously approved these changes Feb 12, 2020
stan/math/rev/fun/inverse.hpp Outdated Show resolved Hide resolved
@bob-carpenter
Copy link
Contributor

I approved because I don't think it's worth changing return style, but I'll let @mcol decide. Feel free to merge as is.

@mcol
Copy link
Contributor Author

mcol commented Feb 12, 2020

Since I didn't think it was worth fixing just that one case, I corrected that pattern everywhere.

@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.9 4.92 1.0 -0.45% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.97 -3.06% slower
eight_schools/eight_schools.stan 0.09 0.09 0.98 -2.03% slower
gp_regr/gp_regr.stan 0.22 0.22 1.0 0.2% faster
irt_2pl/irt_2pl.stan 6.06 6.1 0.99 -0.65% slower
performance.compilation 88.72 87.46 1.01 1.42% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.52 7.47 1.01 0.65% faster
pkpd/one_comp_mm_elim_abs.stan 21.15 20.86 1.01 1.38% faster
sir/sir.stan 89.71 91.5 0.98 -2.0% slower
gp_regr/gen_gp_data.stan 0.04 0.05 0.99 -1.21% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.01 3.0 1.0 0.43% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.32 0.31 1.02 2.43% faster
arK/arK.stan 1.75 1.74 1.0 0.19% faster
arma/arma.stan 0.79 0.79 1.0 -0.37% slower
garch/garch.stan 0.59 0.59 1.0 0.33% faster
Mean result: 0.998370784699

Jenkins Console Log
Blue Ocean
Commit hash: 310f2e1


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

Copy link
Contributor

@bob-carpenter bob-carpenter left a comment

Choose a reason for hiding this comment

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

This looks really great. I'm loving the new { } return syntax.

@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.9 4.85 1.01 0.99% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.98 -2.32% slower
eight_schools/eight_schools.stan 0.09 0.09 0.99 -0.85% slower
gp_regr/gp_regr.stan 0.22 0.22 0.98 -1.65% slower
irt_2pl/irt_2pl.stan 6.08 6.05 1.0 0.41% faster
performance.compilation 88.21 87.23 1.01 1.1% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.48 7.49 1.0 -0.24% slower
pkpd/one_comp_mm_elim_abs.stan 21.6 21.16 1.02 2.03% faster
sir/sir.stan 91.53 92.32 0.99 -0.87% slower
gp_regr/gen_gp_data.stan 0.05 0.05 1.02 1.92% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.03 2.99 1.01 1.13% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.34 0.32 1.08 7.03% faster
arK/arK.stan 1.75 1.73 1.01 0.64% faster
arma/arma.stan 0.79 0.79 1.0 -0.06% slower
garch/garch.stan 0.58 0.58 1.0 -0.32% slower
Mean result: 1.00646858951

Jenkins Console Log
Blue Ocean
Commit hash: 310f2e1


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
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.99 4.88 1.02 2.21% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.94 -6.22% slower
eight_schools/eight_schools.stan 0.09 0.09 1.01 1.44% faster
gp_regr/gp_regr.stan 0.22 0.22 0.96 -4.41% slower
irt_2pl/irt_2pl.stan 6.07 6.06 1.0 0.19% faster
performance.compilation 88.3 87.14 1.01 1.31% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.48 7.47 1.0 0.15% faster
pkpd/one_comp_mm_elim_abs.stan 20.66 21.27 0.97 -2.96% slower
sir/sir.stan 89.94 90.86 0.99 -1.02% slower
gp_regr/gen_gp_data.stan 0.05 0.04 1.02 2.4% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.0 3.0 1.0 -0.05% slower
pkpd/sim_one_comp_mm_elim_abs.stan 0.32 0.32 0.99 -0.54% slower
arK/arK.stan 1.74 1.75 0.99 -0.6% slower
arma/arma.stan 0.8 0.79 1.01 0.86% faster
garch/garch.stan 0.58 0.58 1.0 0.19% faster
Mean result: 0.995832839626

Jenkins Console Log
Blue Ocean
Commit hash: 310f2e1


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
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.88 4.88 1.0 0.05% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.01 1.22% faster
eight_schools/eight_schools.stan 0.09 0.09 0.96 -3.8% slower
gp_regr/gp_regr.stan 0.22 0.22 1.0 0.08% faster
irt_2pl/irt_2pl.stan 6.07 6.08 1.0 -0.04% slower
performance.compilation 88.1 87.08 1.01 1.16% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.48 7.48 1.0 -0.01% slower
pkpd/one_comp_mm_elim_abs.stan 20.72 21.26 0.97 -2.6% slower
sir/sir.stan 89.78 90.96 0.99 -1.31% slower
gp_regr/gen_gp_data.stan 0.05 0.05 0.98 -2.44% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.03 3.01 1.01 0.67% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.31 0.31 1.0 -0.03% slower
arK/arK.stan 1.74 1.73 1.01 0.88% faster
arma/arma.stan 0.79 0.8 0.98 -1.79% slower
garch/garch.stan 0.58 0.59 0.98 -1.64% slower
Mean result: 0.993865823573

Jenkins Console Log
Blue Ocean
Commit hash: 310f2e1


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

@mcol mcol merged commit 5bb32be into develop Feb 13, 2020
@mcol mcol deleted the bugfix/1463-inverse-size-zero branch February 13, 2020 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inverse should have consistent errors on size 0 inputs inverse_spd should have consistent errors on size 0
3 participants