-
-
Notifications
You must be signed in to change notification settings - Fork 193
Flatten /prim and finish flatten #1606
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
Conversation
…-prim # Conflicts: # stan/math/prim.hpp # test/unit/math/fwd/core/std_numeric_limits_test.cpp # test/unit/math/mix/fun/ldexp_test.cpp # test/unit/math/opencl/kernel_generator/unary_function_cl_test.cpp # test/unit/math/opencl/matrix_cl_test.cpp # test/unit/math/opencl/prim/divide_columns_test.cpp # test/unit/math/opencl/rev/sub_block_test.cpp # test/unit/math/opencl/sub_block_test.cpp # test/unit/math/prim/err/is_matching_size_test.cpp # test/unit/math/prim/fun/F32_test.cpp # test/unit/math/prim/fun/array_builder_test.cpp # test/unit/math/prim/fun/autocorrelation_test.cpp # test/unit/math/prim/fun/autocovariance_test.cpp # test/unit/math/prim/fun/cov_exp_quad_test.cpp # test/unit/math/prim/fun/divide_columns_test.cpp # test/unit/math/prim/fun/gp_exponential_cov_test.cpp # test/unit/math/prim/fun/gp_matern32_cov_test.cpp # test/unit/math/prim/fun/gp_matern52_cov_test.cpp # test/unit/math/prim/fun/gp_periodic_cov_test.cpp # test/unit/math/prim/fun/grad_F32_test.cpp # test/unit/math/prim/fun/grad_reg_inc_gamma_test.cpp # test/unit/math/prim/fun/grad_reg_lower_inc_gamma_test.cpp # test/unit/math/prim/fun/log_inv_logit_diff_test.cpp # test/unit/math/prim/fun/log_sum_exp_test.cpp # test/unit/math/prim/fun/matrix_exp_2x2_test.cpp # test/unit/math/prim/fun/matrix_exp_action_handler_test.cpp # test/unit/math/prim/fun/matrix_exp_multiply_test.cpp # test/unit/math/prim/fun/matrix_exp_pade_test.cpp # test/unit/math/prim/fun/promote_elements_test.cpp # test/unit/math/prim/fun/promote_scalar_test.cpp # test/unit/math/prim/fun/promote_scalar_type_test.cpp # test/unit/math/prim/fun/scale_matrix_exp_multiply_test.cpp # test/unit/math/prim/fun/sqrt_test.cpp # test/unit/math/prim/fun/value_of_rec_test.cpp # test/unit/math/prim/fun/value_of_test.cpp # test/unit/math/prim/meta/is_eigen_test.cpp # test/unit/math/prim/meta/is_std_vector_test.cpp # test/unit/math/prim/meta/seq_view_test.cpp # test/unit/math/prim/prob/dirichlet_log_test.cpp # test/unit/math/rev/core/var_test.cpp # test/unit/math/rev/prob/bernoulli_logit_glm_lpmf_test.cpp # test/unit/math/rev/prob/categorical_logit_glm_lpmf_test.cpp # test/unit/math/rev/prob/neg_binomial_2_log_glm_lpmf_test.cpp # test/unit/math/rev/prob/normal_id_glm_lpdf_test.cpp # test/unit/math/rev/prob/poisson_log_glm_lpmf_test.cpp
# Conflicts: # stan/math/fwd/fun/log_sum_exp.hpp # stan/math/fwd/fun/unit_vector_constrain.hpp # stan/math/opencl/prim/bernoulli_logit_glm_lpmf.hpp # stan/math/opencl/prim/categorical_logit_glm_lpmf.hpp # stan/math/opencl/prim/neg_binomial_2_log_glm_lpmf.hpp # stan/math/opencl/prim/normal_id_glm_lpdf.hpp # stan/math/opencl/prim/ordered_logistic_glm_lpmf.hpp # stan/math/opencl/prim/poisson_log_glm_lpmf.hpp # stan/math/prim/arr.hpp # stan/math/prim/arr/fun/common_type.hpp # stan/math/prim/arr/fun/fill.hpp # stan/math/prim/arr/fun/log_sum_exp.hpp # stan/math/prim/arr/fun/promote_elements.hpp # stan/math/prim/arr/fun/promote_scalar.hpp # stan/math/prim/arr/fun/value_of.hpp # stan/math/prim/arr/fun/value_of_rec.hpp # stan/math/prim/err/check_symmetric.hpp # stan/math/prim/err/check_unit_vector.hpp # stan/math/prim/fun/LDLT_factor.hpp # stan/math/prim/fun/Phi.hpp # stan/math/prim/fun/Phi_approx.hpp # stan/math/prim/fun/acosh.hpp # stan/math/prim/fun/atanh.hpp # stan/math/prim/fun/chol2inv.hpp # stan/math/prim/fun/cholesky_corr_constrain.hpp # stan/math/prim/fun/cholesky_corr_free.hpp # stan/math/prim/fun/corr_matrix_constrain.hpp # stan/math/prim/fun/cov_matrix_constrain.hpp # stan/math/prim/fun/cov_matrix_constrain_lkj.hpp # stan/math/prim/fun/digamma.hpp # stan/math/prim/fun/distance.hpp # stan/math/prim/fun/divide_columns.hpp # stan/math/prim/fun/factor_U.hpp # stan/math/prim/fun/gp_dot_prod_cov.hpp # stan/math/prim/fun/gp_exp_quad_cov.hpp # stan/math/prim/fun/gp_periodic_cov.hpp # stan/math/prim/fun/inv_Phi.hpp # stan/math/prim/fun/inv_logit.hpp # stan/math/prim/fun/inv_sqrt.hpp # stan/math/prim/fun/inv_square.hpp # stan/math/prim/fun/lgamma.hpp # stan/math/prim/fun/log1m.hpp # stan/math/prim/fun/log1m_exp.hpp # stan/math/prim/fun/log1m_inv_logit.hpp # stan/math/prim/fun/log1p.hpp # stan/math/prim/fun/log1p_exp.hpp # stan/math/prim/fun/log2.hpp # stan/math/prim/fun/log_inv_logit.hpp # stan/math/prim/fun/log_mix.hpp # stan/math/prim/fun/log_sum_exp.hpp # stan/math/prim/fun/matrix_exp_2x2.hpp # stan/math/prim/fun/max.hpp # stan/math/prim/fun/min.hpp # stan/math/prim/fun/multiply_lower_tri_self_transpose.hpp # stan/math/prim/fun/read_corr_L.hpp # stan/math/prim/fun/read_cov_L.hpp # stan/math/prim/fun/sd.hpp # stan/math/prim/fun/simplex_constrain.hpp # stan/math/prim/fun/simplex_free.hpp # stan/math/prim/fun/squared_distance.hpp # stan/math/prim/fun/tgamma.hpp # stan/math/prim/fun/trigamma.hpp # stan/math/prim/functor/finite_diff_gradient_auto.hpp # stan/math/prim/mat/fun/Phi.hpp # stan/math/prim/mat/fun/Phi_approx.hpp # stan/math/prim/mat/fun/acosh.hpp # stan/math/prim/mat/fun/asinh.hpp # stan/math/prim/mat/fun/atanh.hpp # stan/math/prim/mat/fun/cbrt.hpp # stan/math/prim/mat/fun/digamma.hpp # stan/math/prim/mat/fun/erf.hpp # stan/math/prim/mat/fun/erfc.hpp # stan/math/prim/mat/fun/exp.hpp # stan/math/prim/mat/fun/exp2.hpp # stan/math/prim/mat/fun/expm1.hpp # stan/math/prim/mat/fun/inv.hpp # stan/math/prim/mat/fun/inv_Phi.hpp # stan/math/prim/mat/fun/inv_cloglog.hpp # stan/math/prim/mat/fun/inv_logit.hpp # stan/math/prim/mat/fun/inv_sqrt.hpp # stan/math/prim/mat/fun/inv_square.hpp # stan/math/prim/mat/fun/lgamma.hpp # stan/math/prim/mat/fun/log.hpp # stan/math/prim/mat/fun/log1m.hpp # stan/math/prim/mat/fun/log1m_exp.hpp # stan/math/prim/mat/fun/log1m_inv_logit.hpp # stan/math/prim/mat/fun/log1p.hpp # stan/math/prim/mat/fun/log1p_exp.hpp # stan/math/prim/mat/fun/log2.hpp # stan/math/prim/mat/fun/log_inv_logit.hpp # stan/math/prim/mat/fun/log_mix.hpp # stan/math/prim/mat/fun/log_sum_exp.hpp # stan/math/prim/mat/fun/logit.hpp # stan/math/prim/mat/fun/promote_scalar.hpp # stan/math/prim/mat/fun/round.hpp # stan/math/prim/mat/fun/square.hpp # stan/math/prim/mat/fun/squared_distance.hpp # stan/math/prim/mat/fun/tgamma.hpp # stan/math/prim/mat/fun/trigamma.hpp # stan/math/prim/mat/fun/trunc.hpp # stan/math/prim/mat/fun/value_of.hpp # stan/math/prim/mat/fun/value_of_rec.hpp # stan/math/prim/prob/bernoulli_logit_glm_lpmf.hpp # stan/math/prim/prob/bernoulli_logit_glm_rng.hpp # stan/math/prim/prob/gaussian_dlm_obs_lpdf.hpp # stan/math/prim/prob/inv_wishart_lpdf.hpp # stan/math/prim/prob/lkj_corr_lpdf.hpp # stan/math/prim/prob/matrix_normal_prec_lpdf.hpp # stan/math/prim/prob/multi_gp_cholesky_lpdf.hpp # stan/math/prim/prob/multi_gp_lpdf.hpp # stan/math/prim/prob/multi_normal_cholesky_lpdf.hpp # stan/math/prim/prob/multi_normal_lpdf.hpp # stan/math/prim/prob/multi_normal_prec_lpdf.hpp # stan/math/prim/prob/neg_binomial_2_log_glm_lpmf.hpp # stan/math/prim/prob/normal_id_glm_lpdf.hpp # stan/math/prim/prob/ordered_logistic_glm_lpmf.hpp # stan/math/prim/prob/ordered_logistic_lpmf.hpp # stan/math/prim/prob/ordered_probit_lpmf.hpp # stan/math/prim/prob/poisson_log_glm_lpmf.hpp # stan/math/prim/prob/wishart_lpdf.hpp # stan/math/prim/scal.hpp # stan/math/rev/fun/cov_exp_quad.hpp # stan/math/rev/fun/dot_product.hpp # stan/math/rev/fun/dot_self.hpp # stan/math/rev/fun/gp_exp_quad_cov.hpp # stan/math/rev/fun/gp_periodic_cov.hpp # stan/math/rev/fun/log_sum_exp.hpp # stan/math/rev/fun/sd.hpp # stan/math/rev/fun/simplex_constrain.hpp # stan/math/rev/fun/squared_distance.hpp # stan/math/rev/functor/adj_jac_apply.hpp
# Conflicts: # stan/math/prim/prob/exp_mod_normal_cdf.hpp # stan/math/prim/prob/exp_mod_normal_lccdf.hpp # stan/math/prim/prob/exp_mod_normal_lcdf.hpp # stan/math/prim/prob/exp_mod_normal_lpdf.hpp
…gs/RELEASE_500/final)
What's the timeline for this? Is it waiting for the freeze first or do we need to push it before it? |
We havent really discussed it. The freeze will start on the 11th (not sure what time, we usually take midnight anywhere on earth- https://www.worldtimeserver.com/time-zones/aoe/ or smth like that). But I am not sure whether to merge this during the freeze or not. Its technically not a bugfix but its not a new feature either. It is a huge PR and once again the review will take some time. Personally I do want to get this in before the next release so that we release the library in a clean state. I dont wont to force it tho. |
…gs/RELEASE_500/final)
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
I would also like to see it make it, but given the size and impact on other PRs it would be more convenient to know that it could go in just after the freeze. @seantalts would that be possible? In any case, I'm starting reviewing it now, let's see how far we can get. |
Thanks Marco! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the first half. Usual bunch of nitpicks, but also 2-3 cases in which a header included itself.
# Conflicts: # stan/math/rev/fun/LDLT_alloc.hpp # stan/math/rev/fun/columns_dot_self.hpp # stan/math/rev/fun/divide.hpp # stan/math/rev/fun/dot_product.hpp # stan/math/rev/fun/dot_self.hpp # stan/math/rev/fun/gp_exp_quad_cov.hpp # stan/math/rev/fun/gp_periodic_cov.hpp # stan/math/rev/fun/initialize_variable.hpp # stan/math/rev/fun/log_determinant_ldlt.hpp # stan/math/rev/fun/log_determinant_spd.hpp # stan/math/rev/fun/matrix_exp_multiply.hpp # stan/math/rev/fun/matrix_power.hpp # stan/math/rev/fun/mdivide_left_ldlt.hpp # stan/math/rev/fun/mdivide_left_spd.hpp # stan/math/rev/fun/mdivide_left_tri.hpp # stan/math/rev/fun/multiply.hpp # stan/math/rev/fun/sd.hpp # stan/math/rev/fun/tcrossprod.hpp # stan/math/rev/fun/trace_inv_quad_form_ldlt.hpp # stan/math/rev/fun/unit_vector_constrain.hpp # stan/math/rev/fun/variance.hpp # stan/math/rev/functor/adj_jac_apply.hpp # stan/math/rev/functor/algebra_solver_fp.hpp # stan/math/rev/functor/algebra_solver_newton.hpp # stan/math/rev/functor/algebra_system.hpp # stan/math/rev/functor/idas_forward_system.hpp # stan/math/rev/functor/idas_system.hpp # stan/math/rev/functor/integrate_1d.hpp # stan/math/rev/functor/kinsol_data.hpp # stan/math/rev/functor/kinsol_solve.hpp
# Conflicts: # stan/math/prim/prob/double_exponential_lccdf.hpp # stan/math/prim/prob/double_exponential_lcdf.hpp # stan/math/prim/prob/double_exponential_lpdf.hpp
I addressed all comments in the first batch. I would definitely wait for at least #1558 to get merged before merging this one. The rest are currently either not ready yet or I would consider as bugfix PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a bit to do, but it should be quite quick.
#1525 is another big one waiting to be merged but that's probably not going to be ready to go for next PR (isn't code freeze today the 11th?). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got to the end! Only reordering stuff here.
Also to fix is the header guard for inverse_softmax.hpp
, and not strictly for this PR the header guard for stan/math/mix/fun/typedefs.hpp
could be fixed too.
I was assuming it wont be ready, but yeah I am happ to wait for that PR too, if needed.
Yes, 11th anywhere on the earth. If this gets approved today I will merge it once the deadline passes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved! All seems in good state, hopefully test will pass and we can finally move to new things! :)
@rok-cesnovar you've done an astounding job through all these PRs, I can't imagine the amount of effort it took to handle such an invasive change and making into reviewable pieces with very minimal disruption to other devs. The simplified codebase will definitely be a much more pleasant place to work on, and we won't miss the old layout!
Agreed, thanks Rok! and thanks @mcol for stepping up to review :) |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
Thanks Marco for all the reviews. You really made this process as smooth as it could have been. |
Summary
Final PR in the flatten of
arr, mat, scal
. This PR flattens the rest of/prim
and removes the checks that checked for arr, scal, mat include order.Tests
/
Checklist
Math issue Closes Flatten scal / arr / mat #937
Copyright holder: Rok Češnovar
By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)