Fix std::array Bug for GPU kernels - For Conservative Transfer - #348
Merged
Conversation
std::array in several structs are introducing silent bugs. In this commit, I focused mainly on conservative field transfer related bugs. It works perfectly fine with kokkkos-openmp but fails (almost all projected values are 0) with kokkos-cuda. There are structs, for example, UniformGrid, which uses std::array data members which are failing silently inside gpu kernels when these are used as, for example Kokkos::View<UniformGrid[1]>. I visually varified the result using the example conservative round-trip python script that uses these codes.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses silent correctness issues when pcms::UniformGrid (and related helper code) is used inside Kokkos GPU/CUDA kernels by replacing std::array members with Kokkos::Array and updating call sites/tests accordingly, with emphasis on conservative transfer workflows.
Changes:
- Replaced
std::arraydata members inpcms::UniformGridwithKokkos::Arrayand adjusted device-friendly logic accordingly. - Updated point search and uniform-grid construction paths to use
Kokkos::Arrayinitializers. - Updated unit tests to use brace patterns compatible with
Kokkos::Arraymember initialization.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/pcms/utility/uniform_grid.h |
Switches UniformGrid storage to Kokkos::Array and updates indexing/utility routines to avoid std::array/std algorithms in kernel-adjacent code. |
src/pcms/utility/arrays.h |
Extends array trait utilities to recognize Kokkos::Array. |
src/pcms/localization/point_search.cpp |
Updates internal bbox/grid construction code to use Kokkos::Array and compatible initialization. |
test/test_uniform_grid.cpp |
Updates UniformGrid initialization in tests for Kokkos::Array members. |
test/test_uniform_grid_field.cpp |
Updates multiple test grid initializations for Kokkos::Array members. |
test/test_point_search.cpp |
Updates grid initialization used with Kokkos::View<Uniform2DGrid[1]> (but currently has a syntax issue that needs fixing). |
test/test_point_evaluator.cpp |
Updates test grid initializations for Kokkos::Array members. |
test/test_omega_h_field2_outofbounds.cpp |
Updates test grid initialization for Kokkos::Array members. |
Suppressed comments (1)
test/test_point_search.cpp:155
- The designated initializer for
edge_lengthis missing=(it uses.edge_length{{...}}), which is inconsistent with other initializations in this PR and is likely to fail to compile under standard C++ designated-initializer rules. Use.edge_length = {{...}}here as well.
grid_h(0) = Uniform2DGrid{
.edge_length{{1, 1}}, .bot_left = {{0, 0}}, .divisions = {{60, 60}}};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Not sure if this is the best solution. It is generated with AI.
Fuad-HH
commented
Aug 2, 2026
Fuad-HH
commented
Aug 2, 2026
Comment on lines
+10
to
+51
| // Mirror pybind11's built-in std::array type caster for Kokkos::Array so that | ||
| // def_readwrite / def_property members work with Python sequences. | ||
| namespace pybind11::detail | ||
| { | ||
| template <typename Type, size_t Size> | ||
| struct type_caster<Kokkos::Array<Type, Size>> | ||
| { | ||
| using ArrayType = Kokkos::Array<Type, Size>; | ||
| using value_conv = make_caster<Type>; | ||
|
|
||
| bool load(handle src, bool convert) | ||
| { | ||
| if (!isinstance<sequence>(src)) | ||
| return false; | ||
| auto seq = reinterpret_borrow<sequence>(src); | ||
| if (seq.size() != Size) | ||
| return false; | ||
| size_t i = 0; | ||
| for (auto it : seq) { | ||
| value_conv conv; | ||
| if (!conv.load(it, convert)) | ||
| return false; | ||
| value[i++] = cast_op<Type>(conv); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| template <typename T> | ||
| static handle cast(T&& src, return_value_policy policy, handle parent) | ||
| { | ||
| list l(Size); | ||
| size_t i = 0; | ||
| for (auto& v : src) | ||
| l[i++] = make_caster<Type>::cast(v, policy, parent); | ||
| return l.release(); | ||
| } | ||
|
|
||
| PYBIND11_TYPE_CASTER(ArrayType, const_name("List[") + | ||
| make_caster<Type>::name + const_name("]")); | ||
| }; | ||
| } // namespace pybind11::detail | ||
|
|
Contributor
Author
There was a problem hiding this comment.
I had to create this type caster for Kokkos::Array support in Python. I am not sure if this is the best way. Added this with AI.
pcms is using c++20 or higher, so this is not needed.
jacobmerson
approved these changes
Aug 2, 2026
Collaborator
|
Thanks for the fixes. This looks ready to merge once the tests finish. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
std::arrayin several structs are introducing silent bugs. In this commit, I focused mainly on conservative field transfer related bugs. I have found that conservative projections work with Kokkos OpenMP backend but fails with Kokkos GPU/CUDA backend.There are structs, for example, UniformGrid, which uses std::array data members which are failing silently inside gpu kernels when these are used as, for example
Kokkos::View<UniformGrid [1]>.I visually verified the result using the example conservative round-trip python script that uses these codes.