There are a number of features of the current generated program code that should be changed going forward. A pull request against this issue would not need to provide one or more of:
[ ] get rid of all those using statements other than using namespace stan::math
[ ] (biggie) get rid of the using namespace stan::math (as is, it could lead to name conflicts with user functions)
[ ] clean up Eigen types
[ ] get rid of typedefs for vector_d, row_vector_d and matrix_d and replace uses with Eigen::VectorXd, Eigen::RowVectorXd, and Eigen::MatrixXd
[ ] * replace Eigen::Matrix<double,Eigen::Dynamic,1> with Eigen::VectorXd (and same for matrices, row vectors)
[ ] get rid of std::ostream* arguments with default = 0 and replace with proper references, std::ostream&
[ ] change all the uses of the stream from pointers to normal variables
[ ] change all the calling functions to make sure there's always a stream (even if it's a null or printstream that's ignored)
[ ] const correctness
[ ] var_context on constructor
[ ] arguments to log_prob
[ ] get rid of 2-arg constructor when interfaces catch up with 3-arg
[ ] generate doxygen-style doc so users can more easily read the generated code
[ ] remove all the old methods that have Eigen replacements (removes final traces of params_i)
[ ] move code gen to Eigen versions
[ ] transform_inits()
[ ] log_prob()
[ ] write_array()
[ ] replace pstream variable name with something like program_print_msgs.
[ ] remove one of constrained_param_names and unconstrained_param_names (these should be the same and it looks like we just have duplicated code)
[ ] remove typedef and figure out how to use the model directly in the interfaces --- the problem now is that I don't think multiple models can be loaded in the same C++ process because you can't have conflicting global typedefs. This is hardcoded into, e.g., CmdStan as return cmdstan::command<stan_model>(argc,argv);. It's used to instantiate the particular model. Maybe if we have a virtual base class and make the model an argument somehow, this won't be a problem? Somehow, we still need to get its name in there and I don't think we can create a class instance by name in C++.
[ ] make current line number local (if possible); again, this is going to be a huge problem if we try to run multiple Stan models in the same process as they'll all be hitting the same counter; we could somewhat mitigate that problem if we only have one model per thread by using thread local declarations of the counter
[ ] rename vars in write_array to constrained_params
[ ] move to two space indents and add spaces after commas---should pass our cpplint
[ ] dive down into the code that's actually being generated and see where functions can be used to replace big blocks of loopy generated code---these should all be replaceable with the appropriate
recursive template functions
[ ] develop a useful abstract base class that can be used as an argument reference for the interfaces
[ ] make sure that the virtuals don't impose any additional computational overhead
Summary:
There are a number of features of the current generated program code that should be changed going forward. A pull request against this issue would not need to provide one or more of:
[ ] get rid of all those using statements other than
using namespace stan::math[ ] (biggie) get rid of the
using namespace stan::math(as is, it could lead to name conflicts with user functions)[ ] clean up Eigen types
vector_d,row_vector_dandmatrix_dand replace uses withEigen::VectorXd,Eigen::RowVectorXd, andEigen::MatrixXdEigen::Matrix<double,Eigen::Dynamic,1>withEigen::VectorXd(and same for matrices, row vectors)[ ] get rid of
std::ostream*arguments with default= 0and replace with proper references,std::ostream&[ ]
constcorrectnessvar_contexton constructorlog_prob[ ] get rid of 2-arg constructor when interfaces catch up with 3-arg
[ ] generate doxygen-style doc so users can more easily read the generated code
[ ] remove all the old methods that have Eigen replacements (removes final traces of
params_i)transform_inits()log_prob()write_array()[ ] replace
pstreamvariable name with something likeprogram_print_msgs.[ ] remove one of
constrained_param_namesandunconstrained_param_names(these should be the same and it looks like we just have duplicated code)[ ] remove typedef and figure out how to use the model directly in the interfaces --- the problem now is that I don't think multiple models can be loaded in the same C++ process because you can't have conflicting global typedefs. This is hardcoded into, e.g., CmdStan as
return cmdstan::command<stan_model>(argc,argv);. It's used to instantiate the particular model. Maybe if we have a virtual base class and make the model an argument somehow, this won't be a problem? Somehow, we still need to get its name in there and I don't think we can create a class instance by name in C++.[ ] make current line number local (if possible); again, this is going to be a huge problem if we try to run multiple Stan models in the same process as they'll all be hitting the same counter; we could somewhat mitigate that problem if we only have one model per thread by using thread local declarations of the counter
[ ] rename vars in write_array to
constrained_params[ ] move to two space indents and add spaces after commas---should pass our cpplint
[ ] dive down into the code that's actually being generated and see where functions can be used to replace big blocks of loopy generated code---these should all be replaceable with the appropriate recursive template functions
[ ] develop a useful abstract base class that can be used as an argument reference for the interfaces
Current Version:
v2.15.0