Add support for Kokkos::parallel_for in the fwd mode
#1022
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1022 +/- ##
=======================================
Coverage 94.12% 94.12%
=======================================
Files 55 55
Lines 8275 8275
=======================================
Hits 7789 7789
Misses 486 486 |
gojakuch
force-pushed
the
kokkos-support
branch
from
dc80470
to
356c5e4
Compare
gojakuch
force-pushed
the
kokkos-support
branch
3 times, most recently
from
f5dd53a
to
e4febde
Compare
gojakuch
force-pushed
the
kokkos-support
branch
2 times, most recently
from
fd3c361
to
9b61df6
Compare
|
@vgvassilev I've finally managed to fix the failing check. can we merge this? |
|
@kliegeois, @brian-kelley, can you take a look? |
vgvassilev
force-pushed
the
kokkos-support
branch
from
9b61df6
to
cbc65c3
Compare
|
if we merge #1038 before this, I think it'd make sense to update the tests in this PR accordingly |
unittests/Kokkos/ParallelFor.cpp
Outdated
|
|
||
| Foo<Kokkos::View<double[5], Kokkos::HostSpace>> f(res, x); | ||
|
|
||
| f(0); |
There was a problem hiding this comment.
Can we add a fixme note to track this being a workaround for a bug in clad?
|
@vgvassilev I will try to review this PR by the end of the week. |
gojakuch
force-pushed
the
kokkos-support
branch
from
cbc65c3
to
b49cad0
Compare
|
Does it work with the |
I haven't tried that. could you give some clues on what exactly you ran? upd: I get it now, I'll take a look |
|
Thanks @gojakuch ! I was going to reply today but that's great that you have been able to reproduce it as well. |
Yes, we should. It's on my todo list but still have not got time to work on it. Basically the idea is to enhance the unittest infrastructure to use the infrastructure to generate a file and then use it as part of the compilation. That should be possible with cmake but will need some time to implement. |
|
@vgvassilev what you suggested sounds good to me but something simpler could work as well. Another project on which I worked (Binder) has tests that takes as input a source code (such as https://github.com/RosettaCommons/binder/blob/master/test/T00.basic.hpp) and then generates the output and compares it to a reference code (such as https://github.com/RosettaCommons/binder/blob/master/test/T00.basic.ref.cpp). If there is a mismatch between the generated code and the reference code, it throws an error. Something like that could be done for Clad. The downside would be that the reference file need to be updated for every new modifications that actually modified the output of previously supported features. However, I like the fact that this approach explicitly tell you what the output should look like. |
@kliegeois well, that's the thing. I understand what you ran but it works out fine on my machine (at least while generating). it produces code without warnings, however the produced code is not compilable on its own, but that's a problem of Clang, as far as I'm aware. it's just that some types are not output to text correctly when resolving template definitions. so I'm not sure it's fixable on the Clad's side. at least that's the only thing happening on my machine from what I can see. correct me if you meant smth else. |
gojakuch
force-pushed
the
kokkos-support
branch
from
b49cad0
to
50d2baf
Compare
|
the last forced push was just me rebasing the PR |
@kliegeois an update: after a quick chat with Vassil earlier today, I'm now convinced that it's fixable on our side. but that falls out of the scope of this PR, so I'll open a separate issue for this and fix it in a separate PR. do you have any suggestions apart from that? if not, I think we can merge this and move on, while I'm working on a fix for that |
|
@gojakuch sure, that sounds good to me, thanks! |
No description provided.