From 3f1f5a8a748d0cf293e25f5da33f46926f56d2be Mon Sep 17 00:00:00 2001 From: rok-cesnovar Date: Thu, 16 Apr 2020 09:16:03 +0200 Subject: [PATCH 1/4] reorder the arguments --- stan/math/fwd/functor/reduce_sum.hpp | 9 ++++--- stan/math/prim/functor/reduce_sum.hpp | 8 +++--- stan/math/rev/functor/reduce_sum.hpp | 5 ++-- .../math/prim/functor/reduce_sum_test.cpp | 4 +-- .../math/prim/functor/reduce_sum_util.hpp | 25 ++++++++++--------- .../unit/math/rev/functor/reduce_sum_test.cpp | 4 +-- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/stan/math/fwd/functor/reduce_sum.hpp b/stan/math/fwd/functor/reduce_sum.hpp index 4ebf14fa245..a50882e1047 100644 --- a/stan/math/fwd/functor/reduce_sum.hpp +++ b/stan/math/fwd/functor/reduce_sum.hpp @@ -38,7 +38,7 @@ struct reduce_sum_impl { * This specialization is not parallelized and works for any autodiff types. * * An instance, f, of `ReduceFunction` should have the signature: - * T f(int start, int end, Vec&& vmapped_subset, std::ostream* msgs, + * T f(Vec&& vmapped_subset, int start, int end, std::ostream* msgs, * Args&&... args) * * `ReduceFunction` must be default constructible without any arguments @@ -73,7 +73,8 @@ struct reduce_sum_impl { } if (auto_partitioning) { - return ReduceFunction()(0, vmapped.size() - 1, std::forward(vmapped), + return ReduceFunction()(std::forward(vmapped), + 0, vmapped.size() - 1, msgs, std::forward(args)...); } else { return_type_t sum = 0.0; @@ -88,8 +89,8 @@ struct reduce_sum_impl { sub_slice.emplace_back(vmapped[i]); } - sum += ReduceFunction()(start, end, std::forward(sub_slice), msgs, - std::forward(args)...); + sum += ReduceFunction()(std::forward(sub_slice), start, end, + msgs, std::forward(args)...); } return sum; } diff --git a/stan/math/prim/functor/reduce_sum.hpp b/stan/math/prim/functor/reduce_sum.hpp index 3acd5ddd2d0..dc63172da04 100644 --- a/stan/math/prim/functor/reduce_sum.hpp +++ b/stan/math/prim/functor/reduce_sum.hpp @@ -85,7 +85,7 @@ struct reduce_sum_impl, sum_ += apply( [&](auto&&... args) { - return ReduceFunction()(r.begin(), r.end() - 1, sub_slice, msgs_, + return ReduceFunction()(sub_slice, r.begin(), r.end() - 1, msgs_, args...); }, args_tuple_); @@ -107,7 +107,7 @@ struct reduce_sum_impl, * arithmetic types. * * ReduceFunction must define an operator() with the same signature as: - * double f(int start, int end, Vec&& vmapped_subset, std::ostream* msgs, + * double f(Vec&& vmapped_subset, int start, int end, std::ostream* msgs, * Args&&... args) * * `ReduceFunction` must be default constructible without any arguments @@ -174,7 +174,7 @@ struct reduce_sum_impl, * This defers to reduce_sum_impl for the appropriate implementation * * ReduceFunction must define an operator() with the same signature as: - * T f(int start, int end, Vec&& vmapped_subset, std::ostream* msgs, Args&&... + * T f(Vec&& vmapped_subset, int start, int end, std::ostream* msgs, Args&&... * args) * * `ReduceFunction` must be default constructible without any arguments @@ -209,7 +209,7 @@ inline auto reduce_sum(Vec&& vmapped, int grainsize, std::ostream* msgs, return return_type(0.0); } - return ReduceFunction()(0, vmapped.size() - 1, std::forward(vmapped), + return ReduceFunction()(std::forward(vmapped), 0, vmapped.size() - 1, msgs, std::forward(args)...); #endif } diff --git a/stan/math/rev/functor/reduce_sum.hpp b/stan/math/rev/functor/reduce_sum.hpp index 4f0d3300fd2..b7c773ae11d 100644 --- a/stan/math/rev/functor/reduce_sum.hpp +++ b/stan/math/rev/functor/reduce_sum.hpp @@ -116,7 +116,8 @@ struct reduce_sum_impl, ReturnType, // Perform calculation var sub_sum_v = apply( [&](auto&&... args) { - return ReduceFunction()(r.begin(), r.end() - 1, local_sub_slice, + return ReduceFunction()(local_sub_slice, + r.begin(), r.end() - 1, msgs_, args...); }, args_tuple_local_copy); @@ -164,7 +165,7 @@ struct reduce_sum_impl, ReturnType, * mode autodiff. * * ReduceFunction must define an operator() with the same signature as: - * var f(int start, int end, Vec&& vmapped_subset, std::ostream* msgs, + * var f(Vec&& vmapped_subset, int start, int end, std::ostream* msgs, * Args&&... args) * * `ReduceFunction` must be default constructible without any arguments diff --git a/test/unit/math/prim/functor/reduce_sum_test.cpp b/test/unit/math/prim/functor/reduce_sum_test.cpp index 463f29a38a9..6e81a76d13e 100644 --- a/test/unit/math/prim/functor/reduce_sum_test.cpp +++ b/test/unit/math/prim/functor/reduce_sum_test.cpp @@ -237,8 +237,8 @@ TEST(StanMathPrim_reduce_sum, sum) { std::vector threading_test_global; struct threading_test_lpdf { template - inline auto operator()(std::size_t start, std::size_t end, - const std::vector&, std::ostream* msgs) const { + inline auto operator()(const std::vector&, std::size_t start, std::size_t end, + std::ostream* msgs) const { threading_test_global[start] = tbb::this_task_arena::current_thread_index(); return stan::return_type_t(0); diff --git a/test/unit/math/prim/functor/reduce_sum_util.hpp b/test/unit/math/prim/functor/reduce_sum_util.hpp index 231f7e32d8b..42097465396 100644 --- a/test/unit/math/prim/functor/reduce_sum_util.hpp +++ b/test/unit/math/prim/functor/reduce_sum_util.hpp @@ -21,8 +21,9 @@ struct count_lpdf { count_lpdf() {} // does the reduction in the sub-slice start to end - inline T operator()(std::size_t start, std::size_t end, - const std::vector& sub_slice, std::ostream* msgs, + inline T operator()(const std::vector& sub_slice, + std::size_t start, std::size_t end, + std::ostream* msgs, const std::vector& lambda, const std::vector& idata) const { return stan::math::poisson_lpmf(sub_slice, lambda[0]); @@ -34,8 +35,8 @@ struct nesting_count_lpdf { nesting_count_lpdf() {} // does the reduction in the sub-slice start to end - inline T operator()(std::size_t start, std::size_t end, - const std::vector& sub_slice, std::ostream* msgs, + inline T operator()(const std::vector& sub_slice, + std::size_t start, std::size_t end, std::ostream* msgs, const std::vector& lambda, const std::vector& idata) const { return stan::math::reduce_sum>(sub_slice, 5, msgs, lambda, @@ -64,7 +65,7 @@ struct sum_lpdf { } template - inline auto operator()(std::size_t start, std::size_t end, T&& sub_slice, + inline auto operator()(T&& sub_slice, std::size_t start, std::size_t end, std::ostream* msgs, Args&&... args) const { using return_type = stan::return_type_t; @@ -77,7 +78,7 @@ struct sum_lpdf { struct start_end_lpdf { template - inline auto operator()(std::size_t start, std::size_t end, T1&&, + inline auto operator()(T1&&, std::size_t start, std::size_t end, std::ostream* msgs, T2&& data) const { stan::return_type_t sum = 0; EXPECT_GE(start, 0); @@ -97,8 +98,9 @@ struct slice_group_count_lpdf { slice_group_count_lpdf() {} // does the reduction in the sub-slice start to end - inline T operator()(std::size_t start, std::size_t end, - const std::vector& lambda_slice, std::ostream* msgs, + inline T operator()(const std::vector& lambda_slice, + std::size_t start, std::size_t end, + std::ostream* msgs, const std::vector& y, const std::vector& gsidx) const { const std::size_t num_groups = end - start + 1; @@ -121,7 +123,7 @@ struct grouped_count_lpdf { // does the reduction in the sub-slice start to end template - inline T operator()(std::size_t start, std::size_t end, VecInt1&& sub_slice, + inline T operator()(VecInt1&& sub_slice, std::size_t start, std::size_t end, std::ostream* msgs, VecT&& lambda, VecInt2&& gidx) const { const std::size_t num_terms = end - start + 1; std::decay_t lambda_slice(num_terms); @@ -168,9 +170,8 @@ auto reduce_sum_sum_lpdf = [](auto&& data, auto&&... args) { template struct static_check_lpdf { template - inline auto operator()(std::size_t start, std::size_t end, - const std::vector&, std::ostream* msgs, - const std::vector& data) const { + inline auto operator()(const std::vector&, std::size_t start, std::size_t end, + std::ostream* msgs, const std::vector& data) const { T sum = 0; EXPECT_LE(end - start + 1, grainsize); for (size_t i = start; i <= end; i++) { diff --git a/test/unit/math/rev/functor/reduce_sum_test.cpp b/test/unit/math/rev/functor/reduce_sum_test.cpp index b2f0212489e..bc01fca19c7 100644 --- a/test/unit/math/rev/functor/reduce_sum_test.cpp +++ b/test/unit/math/rev/functor/reduce_sum_test.cpp @@ -394,8 +394,8 @@ TEST(StanMathRev_reduce_sum, slice_group_gradient) { std::vector threading_test_global; struct threading_test_lpdf { template - inline auto operator()(std::size_t start, std::size_t end, - const std::vector&, std::ostream* msgs) const { + inline auto operator()(const std::vector&, std::size_t start, + std::size_t end, std::ostream* msgs) const { threading_test_global[start] = tbb::this_task_arena::current_thread_index(); return stan::return_type_t(0); From f2c75050422e91e65473b461e09818901281305b Mon Sep 17 00:00:00 2001 From: Stan Jenkins Date: Thu, 16 Apr 2020 03:31:09 -0400 Subject: [PATCH 2/4] [Jenkins] auto-formatting by clang-format version 6.0.0 --- stan/math/fwd/functor/reduce_sum.hpp | 7 +++---- stan/math/rev/functor/reduce_sum.hpp | 3 +-- .../math/prim/functor/reduce_sum_test.cpp | 4 ++-- .../math/prim/functor/reduce_sum_util.hpp | 19 +++++++++---------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/stan/math/fwd/functor/reduce_sum.hpp b/stan/math/fwd/functor/reduce_sum.hpp index a50882e1047..7bcacf958c2 100644 --- a/stan/math/fwd/functor/reduce_sum.hpp +++ b/stan/math/fwd/functor/reduce_sum.hpp @@ -73,8 +73,7 @@ struct reduce_sum_impl { } if (auto_partitioning) { - return ReduceFunction()(std::forward(vmapped), - 0, vmapped.size() - 1, + return ReduceFunction()(std::forward(vmapped), 0, vmapped.size() - 1, msgs, std::forward(args)...); } else { return_type_t sum = 0.0; @@ -89,8 +88,8 @@ struct reduce_sum_impl { sub_slice.emplace_back(vmapped[i]); } - sum += ReduceFunction()(std::forward(sub_slice), start, end, - msgs, std::forward(args)...); + sum += ReduceFunction()(std::forward(sub_slice), start, end, msgs, + std::forward(args)...); } return sum; } diff --git a/stan/math/rev/functor/reduce_sum.hpp b/stan/math/rev/functor/reduce_sum.hpp index b7c773ae11d..f0931060b7c 100644 --- a/stan/math/rev/functor/reduce_sum.hpp +++ b/stan/math/rev/functor/reduce_sum.hpp @@ -116,8 +116,7 @@ struct reduce_sum_impl, ReturnType, // Perform calculation var sub_sum_v = apply( [&](auto&&... args) { - return ReduceFunction()(local_sub_slice, - r.begin(), r.end() - 1, + return ReduceFunction()(local_sub_slice, r.begin(), r.end() - 1, msgs_, args...); }, args_tuple_local_copy); diff --git a/test/unit/math/prim/functor/reduce_sum_test.cpp b/test/unit/math/prim/functor/reduce_sum_test.cpp index 6e81a76d13e..f86389d4eff 100644 --- a/test/unit/math/prim/functor/reduce_sum_test.cpp +++ b/test/unit/math/prim/functor/reduce_sum_test.cpp @@ -237,8 +237,8 @@ TEST(StanMathPrim_reduce_sum, sum) { std::vector threading_test_global; struct threading_test_lpdf { template - inline auto operator()(const std::vector&, std::size_t start, std::size_t end, - std::ostream* msgs) const { + inline auto operator()(const std::vector&, std::size_t start, + std::size_t end, std::ostream* msgs) const { threading_test_global[start] = tbb::this_task_arena::current_thread_index(); return stan::return_type_t(0); diff --git a/test/unit/math/prim/functor/reduce_sum_util.hpp b/test/unit/math/prim/functor/reduce_sum_util.hpp index 42097465396..d0b93108b27 100644 --- a/test/unit/math/prim/functor/reduce_sum_util.hpp +++ b/test/unit/math/prim/functor/reduce_sum_util.hpp @@ -21,9 +21,8 @@ struct count_lpdf { count_lpdf() {} // does the reduction in the sub-slice start to end - inline T operator()(const std::vector& sub_slice, - std::size_t start, std::size_t end, - std::ostream* msgs, + inline T operator()(const std::vector& sub_slice, std::size_t start, + std::size_t end, std::ostream* msgs, const std::vector& lambda, const std::vector& idata) const { return stan::math::poisson_lpmf(sub_slice, lambda[0]); @@ -35,8 +34,8 @@ struct nesting_count_lpdf { nesting_count_lpdf() {} // does the reduction in the sub-slice start to end - inline T operator()(const std::vector& sub_slice, - std::size_t start, std::size_t end, std::ostream* msgs, + inline T operator()(const std::vector& sub_slice, std::size_t start, + std::size_t end, std::ostream* msgs, const std::vector& lambda, const std::vector& idata) const { return stan::math::reduce_sum>(sub_slice, 5, msgs, lambda, @@ -98,9 +97,8 @@ struct slice_group_count_lpdf { slice_group_count_lpdf() {} // does the reduction in the sub-slice start to end - inline T operator()(const std::vector& lambda_slice, - std::size_t start, std::size_t end, - std::ostream* msgs, + inline T operator()(const std::vector& lambda_slice, std::size_t start, + std::size_t end, std::ostream* msgs, const std::vector& y, const std::vector& gsidx) const { const std::size_t num_groups = end - start + 1; @@ -170,8 +168,9 @@ auto reduce_sum_sum_lpdf = [](auto&& data, auto&&... args) { template struct static_check_lpdf { template - inline auto operator()(const std::vector&, std::size_t start, std::size_t end, - std::ostream* msgs, const std::vector& data) const { + inline auto operator()(const std::vector&, std::size_t start, + std::size_t end, std::ostream* msgs, + const std::vector& data) const { T sum = 0; EXPECT_LE(end - start + 1, grainsize); for (size_t i = start; i <= end; i++) { From 5f7da15b22c2d77e8622181dabe7e8fde69286c5 Mon Sep 17 00:00:00 2001 From: rok-cesnovar Date: Thu, 16 Apr 2020 10:57:00 +0200 Subject: [PATCH 3/4] fix reduce_sum_static --- stan/math/prim/functor/reduce_sum_static.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stan/math/prim/functor/reduce_sum_static.hpp b/stan/math/prim/functor/reduce_sum_static.hpp index 0411a998b86..c457b3640d6 100644 --- a/stan/math/prim/functor/reduce_sum_static.hpp +++ b/stan/math/prim/functor/reduce_sum_static.hpp @@ -58,7 +58,7 @@ auto reduce_sum_static(Vec&& vmapped, int grainsize, std::ostream* msgs, return return_type(0); } - return ReduceFunction()(0, vmapped.size() - 1, std::forward(vmapped), + return ReduceFunction()(std::forward(vmapped), 0, vmapped.size() - 1, msgs, std::forward(args)...); #endif } From b2ebd1a9e7b9a0ab2d2d7addbc039a25029d5820 Mon Sep 17 00:00:00 2001 From: rok-cesnovar Date: Thu, 16 Apr 2020 11:04:36 +0200 Subject: [PATCH 4/4] fix doc --- stan/math/prim/functor/reduce_sum_static.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stan/math/prim/functor/reduce_sum_static.hpp b/stan/math/prim/functor/reduce_sum_static.hpp index c457b3640d6..235a6d8c1b6 100644 --- a/stan/math/prim/functor/reduce_sum_static.hpp +++ b/stan/math/prim/functor/reduce_sum_static.hpp @@ -21,7 +21,7 @@ namespace math { * This defers to reduce_sum_impl for the appropriate implementation * * ReduceFunction must define an operator() with the same signature as: - * T f(int start, int end, Vec&& vmapped_subset, std::ostream* msgs, Args&&... + * T f(Vec&& vmapped_subset, int start, int end, std::ostream* msgs, Args&&... * args) * * `ReduceFunction` must be default constructible without any arguments