Skip to content

增加集体通信环境parameterized冒烟#9

Open
ghangz wants to merge 1 commit into
MetaX-MACA:mainfrom
ghangz:mengz/mccl-env-parameterized-smoke
Open

增加集体通信环境parameterized冒烟#9
ghangz wants to merge 1 commit into
MetaX-MACA:mainfrom
ghangz:mengz/mccl-env-parameterized-smoke

Conversation

@ghangz

@ghangz ghangz commented Jun 10, 2026

Copy link
Copy Markdown

这次改动补上了集体通信环境parameterized冒烟,主要是为了解决集体通信测试与结果整理流程里相关信息不够集中、人工整理成本较高的问题,让日常排查、验证和结果归档更直接。

实现上补充了对应工具或脚本逻辑,同时尽量保持现有用法不变,避免影响已有流程。

这一分支已经在沐曦算力环境完成实际验证,相关检查均已通过,现提交合入。

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

Copy link
Copy Markdown

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 replaces hardcoded parameters in the mccl.sh script with configurable environment variables (with default values) and adds a log statement to print these parameters. The review feedback suggests wrapping several shell variables in double quotes to prevent word splitting issues when paths or values contain spaces, which improves the script's robustness.

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 mccl.sh
echo -n "The test is ${BENCH}, the maca version is " && realpath ${MACA_PATH}
${MACA_PATH}/ompi/bin/mpirun -np ${MPI_PROCESS_NUM} ${MPI_RUN_OPT} ${TEST_DIR}/${BENCH} -b 1K -e 1G -d bfloat16 -f 2 -g 1 -n 10
echo "MCCL params: min=${MCCL_MIN_BYTES} max=${MCCL_MAX_BYTES} dtype=${MCCL_DTYPE} step=${MCCL_STEP_FACTOR} gpus_per_process=${MCCL_GPUS_PER_PROCESS} iters=${MCCL_ITERS}"
${MACA_PATH}/ompi/bin/mpirun -np ${MPI_PROCESS_NUM} ${MPI_RUN_OPT} ${TEST_DIR}/${BENCH} -b "${MCCL_MIN_BYTES}" -e "${MCCL_MAX_BYTES}" -d "${MCCL_DTYPE}" -f "${MCCL_STEP_FACTOR}" -g "${MCCL_GPUS_PER_PROCESS}" -n "${MCCL_ITERS}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

在 Shell 脚本中,未加双引号的变量(如 ${MACA_PATH}、${MPI_PROCESS_NUM} 和 ${TEST_DIR}/${BENCH})在路径或值包含空格时可能会导致单词拆分(word splitting)和非预期的行为。为了提高脚本的健壮性,建议对这些变量进行双引号保护。注意:${MPI_RUN_OPT} 包含多个独立的命令行参数,如果对其加双引号会使其被视为单个参数传给 mpirun 导致出错,因此这里保持其不加双引号是正确的。

Suggested change
${MACA_PATH}/ompi/bin/mpirun -np ${MPI_PROCESS_NUM} ${MPI_RUN_OPT} ${TEST_DIR}/${BENCH} -b "${MCCL_MIN_BYTES}" -e "${MCCL_MAX_BYTES}" -d "${MCCL_DTYPE}" -f "${MCCL_STEP_FACTOR}" -g "${MCCL_GPUS_PER_PROCESS}" -n "${MCCL_ITERS}"
"${MACA_PATH}/ompi/bin/mpirun" -np "${MPI_PROCESS_NUM}" ${MPI_RUN_OPT} "${TEST_DIR}/${BENCH}" -b "${MCCL_MIN_BYTES}" -e "${MCCL_MAX_BYTES}" -d "${MCCL_DTYPE}" -f "${MCCL_STEP_FACTOR}" -g "${MCCL_GPUS_PER_PROCESS}" -n "${MCCL_ITERS}"

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