Skip to content

Revert "Feature/elementwise checks revert revert" #2147

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

Merged

Conversation

bbbales2
Copy link
Member

@bbbales2 bbbales2 commented Oct 9, 2020

This reverts #2007 (which was originally #1798)

We've gotta revert the elementwise checks again. They are not quite fast enough. We were seeing across the board 10% hits to performance on non-vectorized unmodified lpdfs between develop yesterday (#50d56cc7) and a commit from August 12th (#3492945).

Code for benchmarking is here

The results were August 12th:

-------------------------------------------------------------------------------------------------
Benchmark                                                       Time             CPU   Iterations
-------------------------------------------------------------------------------------------------
uniform_custom_nonvectorized/1024/manual_time             72990 ns       101277 ns         9354
uniform_custom_nonvectorized/1024/manual_time             73136 ns       101377 ns         9354
uniform_custom_nonvectorized/1024/manual_time             73466 ns       101845 ns         9354
uniform_custom_nonvectorized/1024/manual_time             73290 ns       101530 ns         9354
uniform_custom_nonvectorized/1024/manual_time             74159 ns       102893 ns         9354
uniform_custom_nonvectorized/1024/manual_time             76370 ns       105780 ns         9354
uniform_custom_nonvectorized/1024/manual_time             73118 ns       101646 ns         9354
uniform_custom_nonvectorized/1024/manual_time             72457 ns       100987 ns         9354

And then develop (October 9th):

-------------------------------------------------------------------------------------------------
Benchmark                                                       Time             CPU   Iterations
-------------------------------------------------------------------------------------------------
uniform_custom_nonvectorized/1024/manual_time             85572 ns       113653 ns         7943
uniform_custom_nonvectorized/1024/manual_time             85884 ns       114090 ns         7943
uniform_custom_nonvectorized/1024/manual_time             86669 ns       115095 ns         7943
uniform_custom_nonvectorized/1024/manual_time             85438 ns       113458 ns         7943
uniform_custom_nonvectorized/1024/manual_time             85413 ns       113455 ns         7943
uniform_custom_nonvectorized/1024/manual_time             87240 ns       115899 ns         7943
uniform_custom_nonvectorized/1024/manual_time             87657 ns       116620 ns         7943
uniform_custom_nonvectorized/1024/manual_time             86515 ns       115197 ns         7943

You can bisect this down and find that #a406e8a7 (August 20th) is slow but #8a67bc7b (August 19th) is fast.

Next time we put this in (we still want these), we'll just need to add overloads so that the basic stuff is fast.

@SteveBronder SteveBronder self-requested a review October 10, 2020 05:39
@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 3.09 3.07 1.01 0.67% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.02 1.54% faster
eight_schools/eight_schools.stan 0.12 0.11 1.02 1.88% faster
gp_regr/gp_regr.stan 0.18 0.18 0.97 -2.76% slower
irt_2pl/irt_2pl.stan 5.67 5.72 0.99 -0.95% slower
performance.compilation 92.0 88.39 1.04 3.93% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 9.17 8.44 1.09 7.88% faster
pkpd/one_comp_mm_elim_abs.stan 29.19 29.06 1.0 0.45% faster
sir/sir.stan 132.31 130.86 1.01 1.1% faster
gp_regr/gen_gp_data.stan 0.04 0.05 0.95 -4.73% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.18 3.01 1.06 5.45% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.38 0.39 0.96 -3.76% slower
arK/arK.stan 1.85 1.8 1.03 2.65% faster
arma/arma.stan 0.75 0.72 1.03 3.24% faster
garch/garch.stan 0.62 0.54 1.13 11.59% faster
Mean result: 1.02100884697

Jenkins Console Log
Blue Ocean
Commit hash: c8340b9


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 rok-cesnovar merged commit 56f35eb into develop Oct 10, 2020
@rok-cesnovar rok-cesnovar deleted the revert-2007-feature/elementwise_checks_revert_revert branch October 10, 2020 10: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.

4 participants