[Env] add configs for cbp machines for ASNUM tutorial by tdavidcl · Pull Request #1788 · Shamrock-code/Shamrock

tdavidcl · 2026-04-29T15:10:58Z

No description provided.

github-actions · 2026-04-29T15:11:15Z

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

gemini-code-assist

Code Review

This pull request introduces new environment configurations for CBP Nvidia C4140, DGX A100, and H200x4 machines using AdaptiveCpp (SSCP), alongside path updates for generic CBP machine environments. The review feedback highlights several opportunities for improvement: consolidating duplicated logic across the nearly identical machine configurations to enhance maintainability, removing unused imports and functions in the Python setup scripts, and ensuring robust error propagation in shell scripts by adding '|| return' to CMake commands.

gemini-code-assist · 2026-04-29T15:13:23Z

+import argparse
+import os
+
+import utils.acpp
+import utils.amd_arch
+import utils.cuda_arch
+import utils.envscript
+import utils.sysinfo
+from utils.setuparg import *
+
+NAME = "CBP Nvidia C4140 (4 V100) AdaptiveCpp (SSCP)"
+PATH = "machine/cbp/c4140/acpp-sscp"
+
+
+def is_acpp_already_installed(installfolder):
+    return os.path.isfile(installfolder + "/bin/acpp")
+
+
+def setup(arg: SetupArg, envgen: EnvGen):
+    argv = arg.argv
+    builddir = arg.builddir
+    shamrockdir = arg.shamrockdir
+    buildtype = arg.buildtype
+    lib_mode = arg.lib_mode
+
+    parser = argparse.ArgumentParser(prog=PATH, description=NAME + " env for Shamrock")
+
+    parser.add_argument("--gen", action="store", help="generator to use (ninja or make)")
+
+    args = parser.parse_args(argv)
+
+    gen, gen_opt, cmake_gen, cmake_build_type = utils.sysinfo.select_generator(args, buildtype)
+
+    cmake_extra_args = ""
+
+    envgen.export_list = {
+        "SHAMROCK_DIR": shamrockdir,
+        "BUILD_DIR": builddir,
+        "CMAKE_GENERATOR": cmake_gen,
+        "MAKE_EXEC": gen,
+        "MAKE_OPT": f"({gen_opt})",
+        "CMAKE_OPT": f"({cmake_extra_args})",
+        "SHAMROCK_BUILD_TYPE": f"'{cmake_build_type}'",
+    }
+
+    envgen.ext_script_list = [
+        shamrockdir + "/env/helpers/clone-acpp.sh",
+        shamrockdir + "/env/helpers/pull_reffiles.sh",
+    ]
+
+    envgen.gen_env_file("env_built_acpp.sh")


The setup-env.py and env_built_acpp.sh files for the c4140, dgx, and h200x4 machine configurations are nearly identical. To improve maintainability and adhere to the general rule of refactoring duplicated logic, consider consolidating the common parts into a shared utility or a base configuration script.

References

Refactor duplicated logic into a helper function or lambda to improve readability and maintainability.

gemini-code-assist · 2026-04-29T15:13:23Z

+import argparse
+import os
+
+import utils.acpp
+import utils.amd_arch
+import utils.cuda_arch
+import utils.envscript
+import utils.sysinfo
+from utils.setuparg import *


Several imports are unused, and the star import from utils.setuparg should be replaced with explicit imports to improve clarity and avoid namespace pollution.

Suggested change

import argparse

import os

import utils.acpp

import utils.amd_arch

import utils.cuda_arch

import utils.envscript

import utils.sysinfo

from utils.setuparg import *

import argparse

import utils.sysinfo

from utils.setuparg import SetupArg, EnvGen

gemini-code-assist · 2026-04-29T15:13:23Z

+def is_acpp_already_installed(installfolder):
+    return os.path.isfile(installfolder + "/bin/acpp")
+
+
+def setup(arg: SetupArg, envgen: EnvGen):
+    argv = arg.argv
+    builddir = arg.builddir
+    shamrockdir = arg.shamrockdir
+    buildtype = arg.buildtype
+    lib_mode = arg.lib_mode


The function is_acpp_already_installed and the variable lib_mode are unused in this script and should be removed to keep the codebase clean.

Suggested change

def is_acpp_already_installed(installfolder):

return os.path.isfile(installfolder + "/bin/acpp")

def setup(arg: SetupArg, envgen: EnvGen):

argv = arg.argv

builddir = arg.builddir

shamrockdir = arg.shamrockdir

buildtype = arg.buildtype

lib_mode = arg.lib_mode

def setup(arg: SetupArg, envgen: EnvGen):

argv = arg.argv

builddir = arg.builddir

shamrockdir = arg.shamrockdir

buildtype = arg.buildtype

gemini-code-assist · 2026-04-29T15:13:24Z

+    cmake \
+        -S $SHAMROCK_DIR \
+        -B $BUILD_DIR \
+        -DSHAMROCK_ENABLE_BACKEND=SYCL \
+        -DSYCL_IMPLEMENTATION=ACPPDirect \
+        -DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \
+        -DACPP_PATH="${ACPP_INSTALL_DIR}" \
+        -DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \
+        -DBUILD_TEST=Yes \
+        "${CMAKE_OPT[@]}"


The cmake command in shamconfigure is missing error propagation. Adding || return ensures that if configuration fails, the script stops execution, maintaining consistency with other functions in the environment scripts.

Suggested change

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}"

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}" || return

gemini-code-assist · 2026-04-29T15:13:24Z

+import argparse
+import os
+
+import utils.acpp
+import utils.amd_arch
+import utils.cuda_arch
+import utils.envscript
+import utils.sysinfo
+from utils.setuparg import *


Several imports are unused, and the star import from utils.setuparg should be replaced with explicit imports to improve clarity and avoid namespace pollution.

Suggested change

import argparse

import os

import utils.acpp

import utils.amd_arch

import utils.cuda_arch

import utils.envscript

import utils.sysinfo

from utils.setuparg import *

import argparse

import utils.sysinfo

from utils.setuparg import SetupArg, EnvGen

gemini-code-assist · 2026-04-29T15:13:24Z

+def is_acpp_already_installed(installfolder):
+    return os.path.isfile(installfolder + "/bin/acpp")
+
+
+def setup(arg: SetupArg, envgen: EnvGen):
+    argv = arg.argv
+    builddir = arg.builddir
+    shamrockdir = arg.shamrockdir
+    buildtype = arg.buildtype
+    lib_mode = arg.lib_mode


The function is_acpp_already_installed and the variable lib_mode are unused in this script and should be removed to keep the codebase clean.

Suggested change

def is_acpp_already_installed(installfolder):

return os.path.isfile(installfolder + "/bin/acpp")

def setup(arg: SetupArg, envgen: EnvGen):

argv = arg.argv

builddir = arg.builddir

shamrockdir = arg.shamrockdir

buildtype = arg.buildtype

lib_mode = arg.lib_mode

def setup(arg: SetupArg, envgen: EnvGen):

argv = arg.argv

builddir = arg.builddir

shamrockdir = arg.shamrockdir

buildtype = arg.buildtype

gemini-code-assist · 2026-04-29T15:13:24Z

+    cmake \
+        -S $SHAMROCK_DIR \
+        -B $BUILD_DIR \
+        -DSHAMROCK_ENABLE_BACKEND=SYCL \
+        -DSYCL_IMPLEMENTATION=ACPPDirect \
+        -DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \
+        -DACPP_PATH="${ACPP_INSTALL_DIR}" \
+        -DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \
+        -DBUILD_TEST=Yes \
+        "${CMAKE_OPT[@]}"


The cmake command in shamconfigure is missing error propagation. Adding || return ensures that if configuration fails, the script stops execution, maintaining consistency with other functions in the environment scripts.

Suggested change

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}"

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}" || return

gemini-code-assist · 2026-04-29T15:13:24Z

+import argparse
+import os
+
+import utils.acpp
+import utils.amd_arch
+import utils.cuda_arch
+import utils.envscript
+import utils.sysinfo
+from utils.setuparg import *


Several imports are unused, and the star import from utils.setuparg should be replaced with explicit imports to improve clarity and avoid namespace pollution.

Suggested change

import argparse

import os

import utils.acpp

import utils.amd_arch

import utils.cuda_arch

import utils.envscript

import utils.sysinfo

from utils.setuparg import *

import argparse

import utils.sysinfo

from utils.setuparg import SetupArg, EnvGen

gemini-code-assist · 2026-04-29T15:13:24Z

+def is_acpp_already_installed(installfolder):
+    return os.path.isfile(installfolder + "/bin/acpp")
+
+
+def setup(arg: SetupArg, envgen: EnvGen):
+    argv = arg.argv
+    builddir = arg.builddir
+    shamrockdir = arg.shamrockdir
+    buildtype = arg.buildtype
+    lib_mode = arg.lib_mode


The function is_acpp_already_installed and the variable lib_mode are unused in this script and should be removed to keep the codebase clean.

Suggested change

def is_acpp_already_installed(installfolder):

return os.path.isfile(installfolder + "/bin/acpp")

def setup(arg: SetupArg, envgen: EnvGen):

argv = arg.argv

builddir = arg.builddir

shamrockdir = arg.shamrockdir

buildtype = arg.buildtype

lib_mode = arg.lib_mode

def setup(arg: SetupArg, envgen: EnvGen):

argv = arg.argv

builddir = arg.builddir

shamrockdir = arg.shamrockdir

buildtype = arg.buildtype

gemini-code-assist · 2026-04-29T15:13:24Z

+    cmake \
+        -S $SHAMROCK_DIR \
+        -B $BUILD_DIR \
+        -DSHAMROCK_ENABLE_BACKEND=SYCL \
+        -DSYCL_IMPLEMENTATION=ACPPDirect \
+        -DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \
+        -DACPP_PATH="${ACPP_INSTALL_DIR}" \
+        -DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \
+        -DBUILD_TEST=Yes \
+        "${CMAKE_OPT[@]}"


The cmake command in shamconfigure is missing error propagation. Adding || return ensures that if configuration fails, the script stops execution, maintaining consistency with other functions in the environment scripts.

Suggested change

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}"

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}" || return

github-actions · 2026-04-29T17:19:54Z

Workflow report

workflow report corresponding to commit 4264d24
Commiter email is [email protected]
GitHub page artifact URL GitHub page artifact link (can expire)

Pre-commit check report

Pre-commit check: ✅

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for broken symlinks................................................Passed
check yaml...............................................................Passed
detect private key.......................................................Passed
No-tabs checker..........................................................Passed
Tabs remover.............................................................Passed
cmake-format.............................................................Passed
Validate GitHub Workflows................................................Passed
clang-format.............................................................Passed
ruff check...............................................................Passed
ruff format..............................................................Passed
Check doxygen headers....................................................Passed
Check license headers....................................................Passed
Check #pragma once.......................................................Passed
Check SYCL #include......................................................Passed
No ssh in git submodules remote..........................................Passed
No UTF-8 in files (except for authors)...................................Passed

Test pipeline can run.

Clang-tidy diff report

No relevant changes found.
Well done!

You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review.

Doxygen diff with `main`

Removed warnings : 0
New warnings : 0
Warnings count : 8232 → 8232 (0.0%)

Detailed changes :

tdavidcl added 3 commits April 29, 2026 16:25

[Env] preconfigured env for dgx

e0f57db

fix

96e6785

add other nodes

4264d24

gemini-code-assist Bot reviewed Apr 29, 2026

View reviewed changes

tdavidcl merged commit 5d02d30 into Shamrock-code:main Apr 29, 2026
77 checks passed

tdavidcl deleted the cbp-configs branch April 29, 2026 17:42

Conversation

tdavidcl commented Apr 29, 2026

Uh oh!

github-actions Bot commented Apr 29, 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 Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 29, 2026

Workflow report

Pre-commit check report

Clang-tidy diff report

Doxygen diff with main

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Doxygen diff with `main`