vgvassilev / clad

clad -- automatic differentiation for C/C++
GNU Lesser General Public License v3.0
291 stars 122 forks source link

Add support for `Kokkos::parallel_reduce` in the fwd mode #1056

Closed gojakuch closed 2 months ago

github-actions[bot] commented 2 months ago

clang-tidy review says "All clean, LGTM! :+1:"

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.34%. Comparing base (6f4b081) to head (f59ec7d). Report is 4 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vgvassilev/clad/pull/1056/graphs/tree.svg?width=650&height=150&src=pr&token=9f6Q4em8hE&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev)](https://app.codecov.io/gh/vgvassilev/clad/pull/1056?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev) ```diff @@ Coverage Diff @@ ## master #1056 +/- ## ======================================= Coverage 94.34% 94.34% ======================================= Files 55 55 Lines 8311 8311 ======================================= Hits 7841 7841 Misses 470 470 ```
github-actions[bot] commented 2 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 2 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 2 months ago

clang-tidy review says "All clean, LGTM! :+1:"

gojakuch commented 2 months ago

I'm marking this as ready for review, although I'll try to resolve a couple more things here before merging. this covers most possible cases (everything not covered by this PR is tracked in the comments in corresponding parts of the code for now). there are multiple commits in this PR, because there were several things that had to be done here, but since it's all Kokkos-related and reviews normally take some time, I put them all into this single PR.

gojakuch commented 2 months ago

the reason I haven't covered all the possible cases of parallel_reduce here yet is that I have no idea how to make it work with arbitrary number of reduce parameters in the parallel_reduce call, since we need to define a pushforward that would take twice as many parameters of correct types for this case. not sure how to go about this for now

vgvassilev commented 2 months ago

@kliegeois, can you take a look?

kliegeois commented 2 months ago

I will look into this PR very soon.

kliegeois commented 2 months ago

I tried the PR locally and it generated I think what is expected. I run into a segfault though but I think this is due to a version mismatch. Otherwise the PR looks good to me, thanks!

Have you started looking at the lambda as well?

gojakuch commented 2 months ago

@kliegeois thanks for the approval

Have you started looking at the lambda as well?

yes, but lambdas need some general rework in Clad. that's on my list though. I've opened some issues to track progress on that and I'll get to it soon.