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

CombinedReducers MDRange support #3395

Merged
merged 3 commits into from
Sep 22, 2020

Conversation

dhollman
Copy link

Implement feature request in #3394

@dhollman
Copy link
Author

dhollman commented Sep 17, 2020

@dalg24 I have no idea why clang-format changed some other things in the file that I didn't touch. Any thoughts? Never mind; I figured it out.

Implement feature request in kokkos#3394
@dhollman dhollman force-pushed the issue-3394-mdrange-combined-reducers branch from 9d36a90 to 703b226 Compare September 17, 2020 21:37
@dhollman dhollman requested review from DavidPoliakoff, jrmadsen and nliber and removed request for DavidPoliakoff September 17, 2020 21:39
@dhollman dhollman force-pushed the issue-3394-mdrange-combined-reducers branch from d032d14 to 95de112 Compare September 17, 2020 22:53
It was added for CombinedReducers MDRangePolicy changes, but
I don't need it if I'm going to have to use SFINAE anyway.
@dhollman dhollman force-pushed the issue-3394-mdrange-combined-reducers branch from 95de112 to d963c01 Compare September 17, 2020 22:55
Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

Why aren't you taking advantage of FTAD?

core/src/impl/Kokkos_Combined_Reducer.hpp Show resolved Hide resolved
core/src/impl/Kokkos_Combined_Reducer.hpp Show resolved Hide resolved
@dalg24
Copy link
Member

dalg24 commented Sep 18, 2020

Why aren't you taking advantage of FTAD?

I pulled your changes and I see that my suggestion compiles but produces a bug. Please educate me.

@crtrott
Copy link
Member

crtrott commented Sep 18, 2020

OK I don't understand it either :-) @nliber @dhollman so if you could educate us what is going on maybe with a smaller example that would be awesome!

@masterleinad
Copy link
Contributor

/Users/travis/build/kokkos/kokkos/core/unit_test/TestReduce.hpp:523:63: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
      Kokkos::MDRangePolicy<TEST_EXECSPACE, Kokkos::Rank<3>>({0, 0, 0},
                                                              ^~~~~~~
                                                              {      }
/Users/travis/build/kokkos/kokkos/core/unit_test/TestReduce.hpp:524:63: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
                                                             {nw, 1, 1}),
                                                              ^~~~~~~~
                                                              {       }

Co-authored-by: Daniel Arndt <arndtd@ornl.gov>
@dhollman
Copy link
Author

@dalg24 @crtrott There's actually nothing really complicated going on here. You can't use FTAD here because you can't deduce a variadic parameter pack that's not at the end of the parameter list, so you have to give the first parameter pack explicitly at the call site.

@crtrott
Copy link
Member

crtrott commented Sep 21, 2020

Ah ok I get it now.

@crtrott crtrott merged commit 35613e6 into kokkos:develop Sep 22, 2020
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.

None yet

4 participants