Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #183 +/- ##
=======================================
Coverage 95.18% 95.18%
=======================================
Files 94 94
Lines 9345 9348 +3
=======================================
+ Hits 8895 8898 +3
Misses 450 450 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
EmilyBourne
left a comment
There was a problem hiding this comment.
I don't think this PR makes sense. The if/throw is found inside Kokkos::deep_copy in the form of a KOKKOS_ASSERT and the added parallelism can be obtained from Kokkos using Kokkos::parallel_deep_copy instead of Kokkos::deep_copy
This would not have the if condition in the OpenMP pragma but it is the function that we will use anyway when we move to GPU.
|
I think it makes sense to move the implementation details into a separate function (here copy_vector) instead of calling the Kokkos:: manually. |
I would agree if there were any implementation details 😅 here you are basically just changing the name of the function call. Burying the Kokkos call gives you less control (e.g. you can no longer pass an ExecutionSpace in order to use asynchronous execution) |
|
We assume all Vectos to have the same execution space (cpu or gpu). The only time where we have cpu <-> gpu transfers will be in the |
Merge Request - GuideLine Checklist
Guideline to check code before resolve WIP and approval, respectively.
As many checkboxes as possible should be ticked.
Checks by code author:
Always to be checked:
If functions were changed or functionality was added:
If new functionality was added:
If new third party software is used:
If new mathematical methods or epidemiological terms are used:
Checks by code reviewer(s):