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

Error out for const scalar return type in reduction #4645

Merged
merged 3 commits into from
Jan 6, 2022

Conversation

masterleinad
Copy link
Contributor

Fixes #2090. This pull request also makes sure that we are calling the intended overload for parallel_reduce, i.e., the return value is only copied if the return type is a View or a reducer.

@masterleinad
Copy link
Contributor Author

The linked example now gives

In file included from /tmp/kokkos-alt/core/src/Kokkos_Parallel.hpp:180:0,
                 from /tmp/kokkos-alt/core/src/Kokkos_OpenMP.hpp:62,
                 from /tmp/kokkos-alt/core/src/decl/Kokkos_Declare_OPENMP.hpp:49,
                 from /tmp/kokkos-alt/example/build_cmake_in_tree/build/kokkos/KokkosCore_Config_DeclareBackend.hpp:47,
                 from /tmp/kokkos-alt/core/src/Kokkos_Core.hpp:56,
                 from /tmp/kokkos-alt/example/build_cmake_in_tree/cmake_example.cpp:1:
/tmp/kokkos-alt/core/src/Kokkos_Parallel_Reduce.hpp: In instantiation of ‘std::enable_if_t<(!(Kokkos::is_view<Arg>::value || Kokkos::is_reducer<ReturnType>::value))> Kokkos::parallel_reduce(const size_t&, const FunctorType&, ReturnType&) [with FunctorType = main()::<lambda(int, int&)>; ReturnType = const int; std::enable_if_t<(!(Kokkos::is_view<Arg>::value || Kokkos::is_reducer<ReturnType>::value))> = void; size_t = long unsigned int]’:
/tmp/kokkos-alt/example/build_cmake_in_tree/cmake_example.cpp:8:68:   required from here
/tmp/kokkos-alt/core/src/Kokkos_Parallel_Reduce.hpp:1778:3: error: static assertion failed: A const return type is only allowed for a View or reducer return type!
   static_assert(
   ^~~~~~~~~~~~~
/tmp/kokkos-alt/core/src/Kokkos_Parallel_Reduce.hpp:1588:22: error: ‘static void Kokkos::Impl::ParallelReduceAdaptor<PolicyType, FunctorType, ReturnType>::execute(const string&, const PolicyType&, const FunctorType&, ReturnType&) [with PolicyType = Kokkos::RangePolicy<Kokkos::OpenMP>; FunctorType = main()::<lambda(int, int&)>; ReturnType = const int; std::__cxx11::string = std::__cxx11::basic_string<char>]’, declared using local type ‘const main()::<lambda(int, int&)>’, is used but never defined [-fpermissive]
   static inline void execute(const std::string& label, const PolicyType& policy,
                      ^~~~~~~
/tmp/kokkos-alt/core/src/Kokkos_Parallel_Reduce.hpp:1588:22: warning: ‘static void Kokkos::Impl::ParallelReduceAdaptor<PolicyType, FunctorType, ReturnType>::execute(const string&, const PolicyType&, const FunctorType&, ReturnType&) [with PolicyType = Kokkos::RangePolicy<Kokkos::OpenMP>; FunctorType = main()::<lambda(int, int&)>; ReturnType = const int]’ used but never defined
CMakeFiles/example.dir/build.make:62: recipe for target 'CMakeFiles/example.dir/cmake_example.cpp.o' failed
make[2]: *** [CMakeFiles/example.dir/cmake_example.cpp.o] Error 1
CMakeFiles/Makefile2:464: recipe for target 'CMakeFiles/example.dir/all' failed

for me.

parallel_reduce(const std::string& label, const PolicyType& policy,
const FunctorType& functor, ReturnType& return_value) {
static_assert(
!std::is_const<ReturnType>::value,
"A const return type is only allowed for a View or reducer return type!");
Copy link
Member

Choose a reason for hiding this comment

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

Did you check in our Wiki how we actually refer to the reducer argument? I feel like "return type" might be confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to "reduction result type".

@masterleinad
Copy link
Contributor Author

I needed to also allow pointers to be passed in as const.

@crtrott crtrott merged commit 8c90000 into kokkos:develop Jan 6, 2022
@masterleinad masterleinad deleted the disallow_const_scalar_reduce branch January 7, 2022 19:56
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.

parallel_reduce with const return value reference doesn't fail like it should
3 participants