ginkgo
ginkgo copied to clipboard
Public common kernels
This PR makes the simple run_kernel
interface public and adds an example for how to use to allow portable kernel execution.
format-rebase!
Formatting rebase introduced changes, see Artifacts here to review them
Error: The following files need to be formatted:
include/ginkgo/ginkgo.hpp
You can find a formatting patch under Artifacts here or run format!
if you have write access to Ginkgo
@upsj Could you also point out which part you need the feedback? like the public part or header structure?
Greeting from the preCICE team! We have some feedback regarding the implementation of this feature:
unified_kernels.cmake
isn't installed, so this requires the sources to be present and basically turns off this feature for packages (incl. spack).
Calling this multiple times per target leads to an error due to the repeated target names.
I suggest to either allow for a SUFFIX
argument to avoid collisions, or to append sources to library targets that already exist using target_sources
.
Setting properties on the generated targets isn't straight forward as the names their target names aren't communicated back to the caller.
I suggest to either add an OUTPUT
argument that returns a list of generated targets or define an interface library which is all generated targets link against. This generated target could either be fixed, such as ${targetname}_GinkgoDeps
, or passed in using an option DEPTARGET mydeptarger
.
Using the OUTPUT
option could look like this:
find_package(Gingko REQUIRED)
ginkgo_add_unified_kernels(preciceCore _ginkgo_kernels
src/mapping/ginkgo_kernels/GinkgoRBFKernels.cpp)
foreach(kernel in LISTS _ginkgo_kernels)
target_include_directories($kernel PRIVATE src)
endforeach()
With a dependency graph like this:
flowchart LR;
preciceCore --> preciceCore_cuda --> |includes| inc(["src/"]);
preciceCore --> preciceCore_hip --> |includes| inc(["src/"]);
preciceCore --> preciceCore_omp --> |includes| inc(["src/"]);
The target option could look like this:
find_package(Gingko REQUIRED)
ginkgo_add_unified_kernels(preciceCore
src/mapping/ginkgo_kernels/GinkgoRBFKernels.cpp)
target_include_directories(preciceCore_GinkgoDeps PRIVATE src)
With a dependency graph like this:
flowchart LR;
preciceCore --> preciceCore_cuda --> preciceCore_GinkgoDeps;
preciceCore --> preciceCore_hip --> preciceCore_GinkgoDeps;
preciceCore --> preciceCore_omp --> preciceCore_GinkgoDeps;
preciceCore_GinkgoDeps .-> |includes| inc(["src/"])
If you like to polish this up, I recommend using cmake_parse_arguments to create a nice interface:
ginkgo_add_unified_kernels(
TARGET preciceCore
RESULTS _ginkgo_kernels
src/mapping/ginkgo_kernels/GinkgoRBFKernels.cpp
)
This will not move forward in the near future, and would require a lot of work to update, so I'll close it and reopen a new one if it turns out we want this after all.