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

Cleanup mdivide_* and tests #1715

Merged
merged 2 commits into from
Feb 16, 2020
Merged

Conversation

mcol
Copy link
Contributor

@mcol mcol commented Feb 15, 2020

Summary

This cleans up the mdivide functions, mainly to use return_type_t instead of promote_common. There are a couple of unused header removed and a correction to the return type declared for the fwd version of mdivide_left_tri_low. Fixes #1714.

Tests

Reworked and expanded so that the left and right versions contain the same tests. Some boundary condition tests will be completed in #1689.

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.93 4.99 0.99 -1.3% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.0 0.44% faster
eight_schools/eight_schools.stan 0.09 0.09 1.01 1.36% faster
gp_regr/gp_regr.stan 0.22 0.22 1.02 1.83% faster
irt_2pl/irt_2pl.stan 6.09 6.07 1.0 0.18% faster
performance.compilation 87.97 87.07 1.01 1.02% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.5 7.43 1.01 0.99% faster
pkpd/one_comp_mm_elim_abs.stan 20.82 22.02 0.95 -5.74% slower
sir/sir.stan 97.45 96.72 1.01 0.75% faster
gp_regr/gen_gp_data.stan 0.05 0.05 1.04 4.04% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 2.99 3.01 0.99 -0.58% slower
pkpd/sim_one_comp_mm_elim_abs.stan 0.32 0.34 0.94 -5.93% slower
arK/arK.stan 1.74 1.74 1.0 0.24% faster
arma/arma.stan 0.79 0.81 0.97 -3.17% slower
garch/garch.stan 0.58 0.59 1.0 -0.39% slower
Mean result: 0.996499834573

Jenkins Console Log
Blue Ocean
Commit hash: ed296c8


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

mcol commented Feb 16, 2020

I have two more PRs I'm preparing that depend on this. @rok-cesnovar Would you mind review this?

@rok-cesnovar
Copy link
Member

Sure. I can do this in a few hours when I will be at my computer.

@@ -13,7 +13,7 @@ namespace stan {
namespace math {

template <typename T, int R1, int C1, int R2, int C2>
inline Eigen::Matrix<fvar<T>, R1, C1> mdivide_left_tri_low(
inline Eigen::Matrix<fvar<T>, R1, C2> mdivide_left_tri_low(
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Member

@rok-cesnovar rok-cesnovar left a comment

Choose a reason for hiding this comment

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

Looks great! Much nicer to read. Thanks for adding the zero size tests.

b));

return Eigen::Matrix<return_type_t<T1, T2>, R1, C1>(A).lu().solve(
Eigen::Matrix<return_type_t<T1, T2>, R2, C2>(b));
Copy link
Member

Choose a reason for hiding this comment

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

Much nicer!

@rok-cesnovar rok-cesnovar merged commit f11c9e0 into develop Feb 16, 2020
@rok-cesnovar rok-cesnovar deleted the cleanup/1714-mdivide-left-right branch February 16, 2020 13:52
@mcol
Copy link
Contributor Author

mcol commented Feb 16, 2020

Thank you!

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.

remove uses of promote_common from mdivide_* functions and improve tests
3 participants