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

Fix/rvalue and vector scalar deserializer #3043

Merged
merged 5 commits into from
May 31, 2021

Conversation

SteveBronder
Copy link
Collaborator

Submission Checklist

  • Run unit tests: ./runTests.py src/test/unit
  • Run cpplint: make cpplint
  • Declare copyright holder and open-source license: see below

Summary

Fixes stan-dev/math#2493 by having rvalue() return back references instead of copies for nil index and std::vector subsetting

Intended Effect

Fixes stan-dev/math#2493

How to Verify

./runTests.py -j28 ./src/test/unit/model/indexing/

Side Effects

Documentation

No new docs

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Steve Bronder

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@rok-cesnovar
Copy link
Member

Thanks again!

Will let @t4c1 review this one, due to Mac machine issues its unlikely we release before Tuesday anyways.

@SteveBronder SteveBronder requested a review from t4c1 May 30, 2021 23:02
Copy link
Collaborator

@t4c1 t4c1 left a comment

Choose a reason for hiding this comment

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

Its great that you fixed it! I just think some of the changes you made are redundant.

src/stan/io/deserializer.hpp Show resolved Hide resolved
src/stan/model/indexing/rvalue.hpp Outdated Show resolved Hide resolved
…ve std::forward from to_ref() in rvalue() for cases it's not necessary
…m:stan-dev/stan into fix/rvalue-and-vector-scalar-deserializer
@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 1.99 1.94 1.03 2.45% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.01 0.01 1.07 6.28% faster
eight_schools/eight_schools.stan 0.07 0.07 1.01 0.87% faster
gp_regr/gp_regr.stan 0.1 0.1 1.01 1.02% faster
irt_2pl/irt_2pl.stan 3.14 3.14 1.0 0.01% faster
performance.compilation 67.11 61.74 1.09 8.01% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 6.16 6.01 1.03 2.48% faster
pkpd/one_comp_mm_elim_abs.stan 18.41 18.15 1.01 1.41% faster
sir/sir.stan 78.88 79.78 0.99 -1.14% slower
gp_regr/gen_gp_data.stan 0.02 0.02 1.04 4.25% faster
garch/garch.stan 0.36 0.32 1.11 9.83% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.26 0.25 1.02 1.52% faster
arK/arK.stan 1.15 1.14 1.01 1.29% faster
arma/arma.stan 0.43 0.43 1.01 0.57% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 2.03 2.01 1.01 0.84% faster
Mean result: 1.02818691013

Jenkins Console Log
Blue Ocean
Commit hash: bfc2867


Machine information ProductName: Mac OS X ProductVersion: 10.15.7 BuildVersion: 19H15

CPU:
Intel(R) Core(TM) i7-8700B CPU @ 3.20GHz

G++:
Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/c++/4.2.1
Apple clang version 12.0.0 (clang-1200.0.32.27)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Clang:
Apple clang version 12.0.0 (clang-1200.0.32.27)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

@rok-cesnovar rok-cesnovar merged commit c6a5b6f into develop May 31, 2021
@rok-cesnovar rok-cesnovar deleted the fix/rvalue-and-vector-scalar-deserializer branch May 31, 2021 15:52
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.

Performance regression in 2.27.0 RC
4 participants