Skip to content

Adds Serializer and virtual functions for new transform inits#3019

Merged
bbbales2 merged 21 commits into
developfrom
feature/serializer
Mar 17, 2021
Merged

Adds Serializer and virtual functions for new transform inits#3019
bbbales2 merged 21 commits into
developfrom
feature/serializer

Conversation

@SteveBronder
Copy link
Copy Markdown
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

This adds a stan::io::serializer class that reads in objects and writes them to a linear array of data. This will be used in the new transform_inits() signature for the model class to serialize the unconstrained parameters.

The only function here is .write() which accepts any Stan type and writes that to the internal reference to a linear vector.

Eigen::MatrixXd xx = Eigen::MatrixXd::Random(10, 10)
Eigen::VectorXd my_vec(100);
stan::io::serializer serializer(my_vec);
serializer.write(xx); // Write xx to my_vec

The new transform inits signature is of the form

void transform_inits(Eigen::VectorXd& input_r, 
                                Eigen::VectorXd& params_r,
                                std::ostream* msgs)

Where input_r is the input parameters values and params_r is an input/output vector of the unconstrained parameters values

The new transform inits will use these classes by wrapping input_r into stan::io::deserializer and params_r into stan::io::serializer, where it can perform the unconstrain as

stan::io::deserializer deserializer(input_r);
stan::io::serializer serializer(params_r);
serializer.write(deserializer.read_free_lub<std::vector<Eigen::MatrixXd>>(lb, ub, J, N, M));

Where J is the size of the array and N, M are the dimensions of the matrix.

Intended Effect

Along with #3018 this will cleanup a lot of the code generated by the Stan compiler

How to Verify

Tests can be run with

./runTests.py ./src/test/unit/io/serializer_varmat_test.cpp ./runTests.py ./src/test/unit/io/serializer_test.cpp ./runTests.py ./src/test/unit/io/serializer_stdvector_test.cpp

These test all the basic types, arrays of those basic types, and var_value<> types with inner values of the basic types

Side Effects

None

Documentation

Documentation is written for the new class and for each method in the class.

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: Ste

@stan-buildbot
Copy link
Copy Markdown
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 3.58 3.47 1.03 2.93% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.97 -3.12% slower
eight_schools/eight_schools.stan 0.11 0.11 1.02 1.97% faster
gp_regr/gp_regr.stan 0.16 0.16 1.03 3.04% faster
irt_2pl/irt_2pl.stan 5.37 5.24 1.02 2.35% faster
performance.compilation 91.42 89.17 1.03 2.46% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 9.04 9.08 1.0 -0.41% slower
pkpd/one_comp_mm_elim_abs.stan 32.34 30.27 1.07 6.38% faster
sir/sir.stan 129.56 129.46 1.0 0.07% faster
gp_regr/gen_gp_data.stan 0.04 0.04 1.0 -0.31% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.1 3.1 1.0 -0.1% slower
pkpd/sim_one_comp_mm_elim_abs.stan 0.4 0.39 1.03 2.95% faster
arK/arK.stan 2.58 2.6 1.0 -0.5% slower
arma/arma.stan 0.63 0.64 0.99 -1.27% slower
garch/garch.stan 0.61 0.6 1.01 0.92% faster
Mean result: 1.01223591044

Jenkins Console Log
Blue Ocean
Commit hash: fdbf461


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
Copy Markdown
Member

@bbbales2 bbbales2 left a comment

Choose a reason for hiding this comment

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

I like the serializer and updated the tests a bit. That part is good.

I left a Q about one of the transform inits.

It seems we're getting warnings in upstream tests about this too that are causing Jenkins to reject this:

'transform_inits' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void transform_inits(const io::var_context& context, ^ src/test/test-models/transformed_data_rng_test.hpp:37:54: note: in instantiation of template class 'stan::model::model_base_crtp' requested here class transformed_data_rng_test_model final : public model_base_crtp { ^

Comment thread src/stan/model/model_base_crtp.hpp Outdated
}

void transform_inits(const io::var_context& context,
Eigen::VectorXd& params_r, std::ostream* msgs) const {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't this version of the function already exist? I can see why the signature below would be new but I do not understand this one

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oddly I'm not seeing it in model_base_crtp but am seeing it in model_base. I think it just needs an override keyword and we'll miss this

@SteveBronder
Copy link
Copy Markdown
Collaborator Author

@bbbales2 I thought we could just add the new signature in this PR but I think the steps here are

  1. Put in this PR and the deserializer PR
  2. Ryan puts in his PR
  3. We then add the new transform_init() signature since it exists in the model and use it in the code

@stan-buildbot
Copy link
Copy Markdown
Contributor


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.07% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.01 1.42% faster
eight_schools/eight_schools.stan 0.11 0.11 1.02 1.78% faster
gp_regr/gp_regr.stan 0.16 0.16 1.0 0.29% faster
irt_2pl/irt_2pl.stan 5.26 5.25 1.0 0.1% faster
performance.compilation 90.89 88.5 1.03 2.63% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 8.89 8.91 1.0 -0.29% slower
pkpd/one_comp_mm_elim_abs.stan 31.05 30.39 1.02 2.13% faster
sir/sir.stan 128.75 129.5 0.99 -0.58% slower
gp_regr/gen_gp_data.stan 0.03 0.03 1.0 -0.37% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.08 3.09 1.0 -0.23% slower
pkpd/sim_one_comp_mm_elim_abs.stan 0.42 0.38 1.09 8.39% faster
arK/arK.stan 1.88 1.95 0.97 -3.27% slower
arma/arma.stan 0.64 0.63 1.02 1.63% faster
garch/garch.stan 0.51 0.51 1.0 -0.01% slower
Mean result: 1.00985436041

Jenkins Console Log
Blue Ocean
Commit hash: a723a4e


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

@SteveBronder
Copy link
Copy Markdown
Collaborator Author

Aight @bbbales2 I think this is good to go!

Copy link
Copy Markdown
Member

@bbbales2 bbbales2 left a comment

Choose a reason for hiding this comment

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

Good!

@rok-cesnovar rok-cesnovar deleted the feature/serializer branch April 2, 2021 18:17
@seantalts
Copy link
Copy Markdown
Member

Are there examples of what the new stanc3 code should look like using this?

Just a note from another thread - we can't change the transform_init method without bumping the major version number of Stan, so I would propose we not plan on that right now.

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.

5 participants