[Patch] migrate SerialPatchTree & PatchtreeField to sham::DeviceBuffer by TApplencourt · Pull Request #1798 · Shamrock-code/Shamrock

TApplencourt · 2026-05-08T14:30:43Z

Continues the sycl::buffer -> sham::DeviceBuffer migration (#490, #672, #1461). Swap the underlying storage in:

PatchtreeField: internal_buf is now std::optional<DeviceBuffer>. allocate() now requires a DeviceScheduler_ptr.
SerialPatchTree: serial_tree_buf and linked_patch_ids_buf become std::optional<DeviceBuffer<...>>. attach_buf/detach_buf API preserved for caller compatibility.
compute_patch_owner: returns DeviceBuffer, kernel rewritten with sham::kernel_call + MultiRef.
make_patch_tree_field: takes DeviceScheduler_ptr instead of sycl::queue, reduction loop migrated to sham::kernel_call.
host_for_each_leafs: uses BufferMirror instead of sycl::host_accessor.

Propagate the DistributedData<DeviceBuffer> change through ReattributeDataUtility (compute_new_pid, extract_elements, reatribute_patch_objects).

Update callers: BasicSPHGhosts, GSPHGhostHandler, SPHSetup, SPHUtilities, GSPHUtilities.

Delete the now-dead reduce_field method and patch_reduc_tree.hpp (PatchFieldReduction had zero callers).

Assisted-by: Claude Opus 4.7 noreply@anthropic.com

Continues the sycl::buffer -> sham::DeviceBuffer migration (Shamrock-code#490, Shamrock-code#672, Shamrock-code#1461). Swap the underlying storage in: - PatchtreeField<T>: internal_buf is now std::optional<DeviceBuffer<T>>. allocate() now requires a DeviceScheduler_ptr. - SerialPatchTree<T>: serial_tree_buf and linked_patch_ids_buf become std::optional<DeviceBuffer<...>>. attach_buf/detach_buf API preserved for caller compatibility. - compute_patch_owner: returns DeviceBuffer<u64>, kernel rewritten with sham::kernel_call + MultiRef. - make_patch_tree_field: takes DeviceScheduler_ptr instead of sycl::queue, reduction loop migrated to sham::kernel_call. - host_for_each_leafs: uses BufferMirror<host> instead of sycl::host_accessor. Propagate the DistributedData<DeviceBuffer<u64>> change through ReattributeDataUtility (compute_new_pid, extract_elements, reatribute_patch_objects). Update callers: BasicSPHGhosts, GSPHGhostHandler, SPHSetup, SPHUtilities, GSPHUtilities. Delete the now-dead reduce_field method and patch_reduc_tree.hpp (PatchFieldReduction had zero callers). Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>

github-actions · 2026-05-08T14:31:00Z

Thanks @TApplencourt for opening this PR!

You can do multiple things directly here:
1 - Comment pre-commit.ci run to run pre-commit checks.
2 - Comment pre-commit.ci autofix to apply fixes.
3 - Add label autofix.ci to fix authorship & pre-commit for every commit made.
4 - Add label light-ci to only trigger a reduced & faster version of the CI (need the full one before merge).
5 - Add label trigger-ci to create an empty commit to trigger the CI.

Once the workflow completes a message will appear displaying informations related to the run.

Also the PR gets automatically reviewed by gemini, you can:
1 - Comment /gemini review to trigger a review
2 - Comment /gemini summary for a summary
3 - Tag it using @gemini-code-assist either in the PR or in review comments on files

TApplencourt · 2026-05-08T14:31:54Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

gemini-code-assist

Code Review

This pull request refactors the codebase to replace SYCL buffers and accessors with sham::DeviceBuffer and BufferMirror abstractions, while transitioning from direct SYCL queue submissions to sham::kernel_call. Key changes include updating PatchtreeField and SerialPatchTree to use std::optionalsham::DeviceBuffer and replacing sycl::host_accessor with mirror_tosham::host() or copy_to_stdvec(). Feedback identifies several performance bottlenecks where data transfers or heap allocations occur within loops. Furthermore, the reviewer pointed out multiple safety regressions where std::optional buffers and scheduler pointers are dereferenced without proper validation, which could lead to runtime crashes.

gemini-code-assist · 2026-05-08T14:34:05Z

+                    auto tree = sptree.serial_tree_buf->template mirror_to<sham::host>();
+                    auto lpid = sptree.linked_patch_ids_buf->template mirror_to<sham::host>();


Creating and destroying BufferMirror objects inside the triple loop over periodic offsets is highly inefficient. Each creation triggers a Device-to-Host copy, and each destruction triggers a Host-to-Device copy (write-back). These mirrors should be moved outside the loops (before line 296) to avoid redundant data transfers.

gemini-code-assist · 2026-05-08T14:34:05Z

+            auto tree = sptree.serial_tree_buf->template mirror_to<sham::host>();
+            auto lpid = sptree.linked_patch_ids_buf->template mirror_to<sham::host>();


Similar to the periodic case, creating mirrors inside the for_each_patch_shift lambda causes repeated data transfers for every shift. These mirrors should be moved outside the for_each_patch_shift call (before line 362).

gemini-code-assist · 2026-05-08T14:34:06Z

                    bool err_id_in_newid = false;
                    {
-                        sycl::host_accessor nid{newid_buf_map.get(id), sycl::read_only};
+                        std::vector<u64> nid = newid_buf_map.get(id).copy_to_stdvec();


copy_to_stdvec() performs a heap allocation and a full buffer copy from device to host. Calling it inside for_each_patchdata is inefficient and can become a performance bottleneck as the number of patches increases. Consider if this logic can be moved to a device kernel or if a more efficient host access method is available.

gemini-code-assist · 2026-05-08T14:34:06Z

                    if (!pdat.is_empty()) {

-                        sycl::host_accessor nid{new_pid.get(current_pid), sycl::read_only};
+                        std::vector<u64> nid = new_pid.get(current_pid).copy_to_stdvec();


Similar to the usage in compute_new_pid, calling copy_to_stdvec() inside a loop over patches is inefficient due to repeated allocations and data transfers.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

tdavidcl · 2026-05-08T21:34:13Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

github-actions · 2026-05-29T17:49:18Z

Workflow report

workflow report corresponding to commit 08d1a1b
Commiter email is timothee.davidcleris@proton.me

Pre-commit check report

Some failures were detected in base source checks checks.
Check the On PR / Linting / Base source checks (pull_request) job in the tests for more detailed output

Suggested changes

Detailed changes :

diff --git a/src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp b/src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp
index 203a79a8..1d691a2e 100644
--- a/src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp
+++ b/src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp
@@ -256,10 +256,9 @@ sham::DeviceBuffer<u64> SerialPatchTree<vec>::compute_patch_owner(
     sham::DeviceScheduler_ptr dev_sched, sham::DeviceBuffer<vec> &position_buffer, u32 len) {
     sham::DeviceBuffer<u64> new_owned_id(len, dev_sched);
 
-
-   if(len == 0){
-   return new_owned_id;
-   }
+    if (len == 0) {
+        return new_owned_id;
+    }
 
     using namespace shamrock::patch;

This PR cherry pick code deletions from #1798 --------- Co-authored-by: tapplencourt <tapplencourt@anl.gov>

for more information, see https://pre-commit.ci

[pre-commit.ci] auto fixes from pre-commit.com hooks

8e0470e

for more information, see https://pre-commit.ci

gemini-code-assist Bot reviewed May 8, 2026

View reviewed changes

TApplencourt and others added 5 commits May 8, 2026 16:37

Update src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp

f44883e

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp

663d24b

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp

039c48e

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp

8b5fdbe

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp

ff85c06

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

170dba2

for more information, see https://pre-commit.ci

tdavidcl added in-review in-testing labels May 11, 2026

tdavidcl added 3 commits May 11, 2026 16:42

Merge branch 'main' into migrate/serial-patch-tree-devicebuffer

72434c1

Merge branch 'main' into migrate/serial-patch-tree-devicebuffer

f957225

Merge branch 'main' into migrate/serial-patch-tree-devicebuffer

199ce84

tdavidcl reviewed May 29, 2026

View reviewed changes

Comment thread src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp

tdavidcl added 2 commits May 29, 2026 19:47

Update src/shamrock/include/shamrock/scheduler/SerialPatchTree.hpp

f1e29d2

Merge branch 'main' into migrate/serial-patch-tree-devicebuffer

08d1a1b

tdavidcl mentioned this pull request May 29, 2026

[Scheduler] cherry pick code deletions from #1798 #1855

Merged

tdavidcl added a commit that referenced this pull request May 30, 2026

[Scheduler] cherry pick code deletions from #1798 (#1855)

3519984

This PR cherry pick code deletions from #1798 --------- Co-authored-by: tapplencourt <tapplencourt@anl.gov>

Merge branch 'main' into migrate/serial-patch-tree-devicebuffer

d051bfd

tdavidcl added the pre-commit.ci autofix label May 30, 2026

pre-commit-ci Bot removed the pre-commit.ci autofix label May 30, 2026

[pre-commit.ci] auto fixes from pre-commit.com hooks

d2c5cac

for more information, see https://pre-commit.ci

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Patch] migrate SerialPatchTree & PatchtreeField to sham::DeviceBuffer#1798

[Patch] migrate SerialPatchTree & PatchtreeField to sham::DeviceBuffer#1798
TApplencourt wants to merge 15 commits into
Shamrock-code:mainfrom
TApplencourt:migrate/serial-patch-tree-devicebuffer

TApplencourt commented May 8, 2026

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

TApplencourt commented May 8, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 8, 2026

Uh oh!

gemini-code-assist Bot May 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot May 8, 2026

Uh oh!

gemini-code-assist Bot May 8, 2026

Uh oh!

tdavidcl commented May 8, 2026

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		auto tree = sptree.serial_tree_buf->template mirror_to<sham::host>();
		auto lpid = sptree.linked_patch_ids_buf->template mirror_to<sham::host>();

Conversation

TApplencourt commented May 8, 2026

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

TApplencourt commented May 8, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

Uh oh!

tdavidcl commented May 8, 2026

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2026

Workflow report

Pre-commit check report

Suggested changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants