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
Allow functors as reducers for nested team parallel reduce #6921
base: develop
Are you sure you want to change the base?
Conversation
56716f9
to
cd00bd8
Compare
…torRange in HostThreadTeam
…torRange for Cuda
…torRange for SYCL
05b6edf
to
684bb69
Compare
Only |
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.
I only commented on the Cuda
backend but the same comments apply to all other backends.
constexpr bool is_reducer_closure = | ||
functor_analysis_type::has_join_member_function && | ||
functor_analysis_type::has_init_member_function; |
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.
constexpr bool is_reducer_closure = | |
functor_analysis_type::has_join_member_function && | |
functor_analysis_type::has_init_member_function; | |
constexpr bool is_reducer_closure = | |
functor_analysis_type::has_join_member_function; |
using ReducerSelector = | ||
typename Kokkos::Impl::if_c<is_reducer_closure, Closure, | ||
Sum<ValueType>>::type; |
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.
Why are you not using functor_analysis_type::Reducer
? Because that type is not compatible with team_reduce
?
Either way, ReducerSelector
is only used below in the !is_reducer_closure
branch. So there is no need for this alias.
using ReducerSelector = | ||
typename Kokkos::Impl::if_c<is_reducer_closure, Closure, | ||
Sum<ValueType>>::type; |
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.
See above.
if constexpr (is_reducer_v<ReducerType>) { | ||
reducer.reference() = tmp2; | ||
})) |
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.
Why aren't we using final
here if defined?
using ReducerSelector = | ||
typename Kokkos::Impl::if_c<is_reducer_closure, Closure, | ||
Sum<ValueType>>::type; |
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.
See above.
KOKKOS_INLINE_FUNCTION | ||
void final(value_type &) const {} |
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.
We should test with a non-trivial final
as well.
Resolves #6317.
This PR allows functors to be used as reducers in nested parallel reduction, specifically for
parallel_reduce
that takes inTeamThreadRange
,ThreadVectorRange
orTeamVectorRange
.This capability is already available for reductions with
Range/MDRange/Team
policies as noted in #6774.To stay consistent with the existing 'functors as reducers' behavior , changes in this PR expects:
ReducerArgument
is not a reducer typeinit()
andjoin()
definedReducerConcept
(i.e. havingtypedef reducer = [ReducerStruct]
is not required)