Skip to content

[AMD] pin rccl-tests to a known-good commit#1693

Open
XinyuJiangCMU wants to merge 1 commit into
radixark:mainfrom
XinyuJiangCMU:pr/rocm72-rccl-pin
Open

[AMD] pin rccl-tests to a known-good commit#1693
XinyuJiangCMU wants to merge 1 commit into
radixark:mainfrom
XinyuJiangCMU:pr/rocm72-rccl-pin

Conversation

@XinyuJiangCMU

@XinyuJiangCMU XinyuJiangCMU commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Co-authored-with: @JessicaJiang-123

ROCm/rocm-systems develop chases NCCL symbols (e.g. NCCL_CTA_POLICY_ZERO) that the base image's RCCL headers don't define, breaking the rccl-tests build. Pin to 746c7b3c and shallow fetch-by-sha instead of tracking develop.

ROCm/rocm-systems develop chases NCCL symbols (e.g. NCCL_CTA_POLICY_ZERO)
that the base image's RCCL headers don't define, breaking the rccl-tests
build. Pin to 746c7b3c and shallow fetch-by-sha instead of tracking develop.

Co-authored-by: Xinyu Jiang <xinyuj2@andrew.cmu.edu>
Co-authored-by: Zhiyao Jiang <jessicajiang324@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates docker/Dockerfile.rocm to pin the rccl-tests repository to a specific commit instead of a branch. Feedback on this change highlights a potential issue where the temporary directory /tmp/rocm-systems is deleted while the shell process is still inside it, which can cause unexpected behavior. It is recommended to change the directory (e.g., to /) before executing the rm -rf command.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread docker/Dockerfile.rocm
Comment on lines +75 to 81
RUN mkdir -p /tmp/rocm-systems && cd /tmp/rocm-systems && git init -q && \
git remote add origin ${RCCL_TESTS_REPO} && \
git fetch --depth 1 origin ${RCCL_TESTS_COMMIT} && git checkout -q FETCH_HEAD && \
make -C ${RCCL_TESTS_PATH} -j$(nproc) \
HIP_HOME=/opt/rocm NCCL_HOME=/opt/rocm GPU_TARGETS=${GPU_ARCH} && \
cp ${RCCL_TESTS_PATH}/build/*_perf /usr/local/bin/ && \
rm -rf /tmp/rocm-systems

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Deleting the current working directory (/tmp/rocm-systems) while the shell process is still inside it can cause unexpected behavior or failures in some environments or storage drivers. It is safer to change the directory (e.g., back to / or the parent directory) before executing rm -rf.

RUN mkdir -p /tmp/rocm-systems && cd /tmp/rocm-systems && git init -q && \
    git remote add origin ${RCCL_TESTS_REPO} && \
    git fetch --depth 1 origin ${RCCL_TESTS_COMMIT} && git checkout -q FETCH_HEAD && \
    make -C ${RCCL_TESTS_PATH} -j$(nproc) \
      HIP_HOME=/opt/rocm NCCL_HOME=/opt/rocm GPU_TARGETS=${GPU_ARCH} && \
    cp ${RCCL_TESTS_PATH}/build/*_perf /usr/local/bin/ && \
    cd / && rm -rf /tmp/rocm-systems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant