[Algs] WIP: Implement a in-place variant of sparse comm #1484

tdavidcl · 2025-12-24T14:14:11Z

No description provided.

gemini-code-assist · 2025-12-24T14:14:25Z

Summary of Changes

Hello @tdavidcl, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the shamalgs library's capabilities for handling sparse collective communications. It introduces a structured and reusable RequestList class for managing MPI requests and establishes a new framework for defining and executing sparse data exchanges through CommMessageInfo and CommTable structures. The changes aim to improve the modularity, maintainability, and efficiency of sparse communication patterns within the codebase.

Highlights

New RequestList Class: Introduced a new RequestList class in RequestList.hpp to centralize the management of MPI requests. This class provides methods for adding new requests, testing their readiness, checking if all are ready, waiting for all to complete, and counting remaining requests.
New Sparse Exchange Mechanism: Added new files sparse_exchange.hpp and sparse_exchange.cpp which define CommMessageInfo and CommTable structs. These structures are designed to manage metadata for sparse communication patterns, including message sizes, sender/receiver ranks, and buffer offsets. Functions build_sparse_exchange_table and sparse_exchange are introduced to facilitate this new communication approach.
Refactoring of sparseXchg.cpp: The existing sparseXchg.cpp file has been refactored to utilize the newly created shamalgs::collective::RequestList class, replacing a previously local and duplicated RequestList struct. This change promotes code reuse and consistency in MPI request handling.
New CommMessage Struct: A CommMessage struct has been added to sparseXchg.hpp to encapsulate details of a communication message, including a byte buffer reference, message offset, size, and sender/receiver ranks.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces new collective communication utilities by extracting a RequestList class into its own header and creating new files (sparse_exchange.hpp, sparse_exchange.cpp) to define CommMessageInfo and CommTable structs, along with functions for building a sparse exchange communication table. The existing sparseXchg.cpp is updated to utilize the new RequestList class. Review comments identify a logical error in the build_sparse_exchange_table function concerning offset validation, suggest simplifying the RequestList::new_request() method, recommend using size_t for loop counters to improve type safety, advise removing an unused CommMessage struct, point out unnecessary #pragma once in a .cpp file, suggest removing a trailing semicolon from a function definition, and recommend marking unused parameters in the sparse_exchange stub to prevent compiler warnings.

src/shamalgs/src/collective/sparse_exchange.cpp

src/shamalgs/include/shamalgs/collective/RequestList.hpp

gemini-code-assist · 2025-12-24T14:16:20Z

src/shamalgs/include/shamalgs/collective/RequestList.hpp

+        std::vector<MPI_Request> &requests() { return rqs; }
+
+        void test_ready() {
+            for (u32 i = 0; i < rqs.size(); i++) {


The loop counter i is of type u32, while rqs.size() returns size_t. This can lead to compiler warnings on 64-bit systems and potential issues if the number of requests exceeds the limit of u32. It's better to use size_t for consistency and correctness.

for (size_t i = 0; i < rqs.size(); i++) {

gemini-code-assist · 2025-12-24T14:16:20Z

src/shamalgs/include/shamalgs/collective/sparseXchg.hpp

+    struct CommMessage {
+        sham::DeviceBuffer<u8> &bytebuffer;
+        size_t message_offset;
+        size_t message_size;
+        i32 sender_rank;
+        i32 receiver_rank;
+    };


The CommMessage struct is defined but appears to be unused anywhere in the changes or the provided file context. This seems to be dead code and should be removed to keep the codebase clean.

gemini-code-assist · 2025-12-24T14:16:20Z

src/shamalgs/src/collective/sparse_exchange.cpp

+//
+// -------------------------------------------------------//
+
+#pragma once


#pragma once is a header guard and is not needed in a .cpp source file. It should be removed.

src/shamalgs/src/collective/sparse_exchange.cpp

github-actions · 2025-12-24T14:16:55Z

Thanks @tdavidcl 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

src/shamrock/include/shamrock/patch/PatchDataField.hpp

github-actions · 2026-01-01T18:06:17Z

Workflow report

workflow report corresponding to commit 33a933c
Commiter email is timothee.davidcleris@proton.me

Light CI is enabled. This will only run the basic tests and not the full tests.
Merging a PR require the job "on PR / all" to pass which is disabled in this case.

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 detailled output

Suggested changes

Detailed changes :

diff --git a/src/shamrock/include/shamrock/patch/PatchDataField.hpp b/src/shamrock/include/shamrock/patch/PatchDataField.hpp
index 921910d8..b467fd34 100644
--- a/src/shamrock/include/shamrock/patch/PatchDataField.hpp
+++ b/src/shamrock/include/shamrock/patch/PatchDataField.hpp
@@ -391,7 +391,7 @@ class PatchDataField {
             // buffer of booleans to store result of the condition
             sham::DeviceBuffer<u32> mask(obj_cnt, dev_sched);
 
-sham::kernel_call(
+            sham::kernel_call(
                 q,
                 sham::MultiRef{buf},
                 sham::MultiRef{mask},

[Algs] Implement a inplace variant of sparse comm

ce709a7

tdavidcl added the draft label Dec 24, 2025

tdavidcl added light-ci trigger-ci labels Dec 24, 2025

gemini-code-assist bot reviewed Dec 24, 2025

View reviewed changes

tdavidcl changed the title ~~[Algs] Implement a in-place variant of sparse comm~~ [Algs] WIP: Implement a in-place variant of sparse comm Dec 24, 2025

tdavidcl removed the trigger-ci label Dec 24, 2025

github-actions bot and others added 16 commits December 24, 2025 14:17

[gh-action] trigger CI with empty commit

df9a29d

update from stack diff

67c05c2

implement more of ti

d1f7b3d

noice

bda7f5e

it works

97dc885

swap comm in the code

1fddeb5

use an alloc cache in exchange

cedccc8

enable persistance

5f0c757

even less allocs

6dad87c

Merge branch 'main' into patch-2025-12-24-11-25

e874faa

fix precommit

229ac8b

Merge branch 'main' into patch-2025-12-24-11-25

520658e

Merge branch 'main' into patch-2025-12-24-11-25

eda394a

fix compile and improve setup perf

3c8c2d3

Merge branch 'main' into patch-2025-12-24-11-25

006aad8

Merge branch 'main' into patch-2025-12-24-11-25

0f431e2

tdavidcl commented Jan 1, 2026

View reviewed changes

src/shamrock/include/shamrock/patch/PatchDataField.hpp Outdated Show resolved Hide resolved

Update src/shamrock/include/shamrock/patch/PatchDataField.hpp

33a933c

tdavidcl added the autofix.ci label Jan 1, 2026

tdavidcl mentioned this pull request Jan 1, 2026

[Algs] Add new inplace sparse communication algorithm #1510

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Algs] WIP: Implement a in-place variant of sparse comm #1484

[Algs] WIP: Implement a in-place variant of sparse comm #1484

Uh oh!

tdavidcl commented Dec 24, 2025

Uh oh!

gemini-code-assist bot commented Dec 24, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Dec 24, 2025

Uh oh!

gemini-code-assist bot Dec 24, 2025

Uh oh!

gemini-code-assist bot Dec 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

Uh oh!

github-actions bot commented Jan 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

[Algs] WIP: Implement a in-place variant of sparse comm #1484

Are you sure you want to change the base?

[Algs] WIP: Implement a in-place variant of sparse comm #1484

Uh oh!

Conversation

tdavidcl commented Dec 24, 2025

Uh oh!

gemini-code-assist bot commented Dec 24, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

Uh oh!

github-actions bot commented Jan 1, 2026

Workflow report

Pre-commit check report

Suggested changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant