rocm: disable gradient_accumulation_fusion on gfx950 in test_qwen2.5_0.5B_gsm8k#1617
rocm: disable gradient_accumulation_fusion on gfx950 in test_qwen2.5_0.5B_gsm8k#1617sreerohi wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces ROCm-specific configurations to the test_qwen2.5_0.5B_gsm8k.py end-to-end test, conditionally disabling the logprobs checker and gradient accumulation fusion when running on ROCm. The reviewer feedback suggests refactoring the string concatenation within the argument list to use f-string ternary expressions instead of mixing implicit and explicit string concatenation, which improves code readability and 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.
| + ("--ci-disable-logprobs-checker " if IS_ROCM else "") | ||
| + "--ci-metric-checker-key eval/gsm8k " |
There was a problem hiding this comment.
Mixing implicit string literal concatenation with explicit + operators inside parenthesized expressions is fragile and can easily lead to syntax errors or unexpected formatting during future modifications. Since Python supports implicit concatenation of f-strings, you can use an f-string ternary expression to keep the entire block as a single, cleanly concatenated string literal sequence.
| + ("--ci-disable-logprobs-checker " if IS_ROCM else "") | |
| + "--ci-metric-checker-key eval/gsm8k " | |
| f"{'--ci-disable-logprobs-checker ' if IS_ROCM else ''}" | |
| "--ci-metric-checker-key eval/gsm8k " |
| + ("--no-gradient-accumulation-fusion " if IS_ROCM else "") | ||
| + "--attention-softmax-in-fp32 " |
There was a problem hiding this comment.
Similarly, using an f-string ternary expression here avoids mixing implicit and explicit string concatenation, keeping the parenthesized string clean and robust against future changes.
| + ("--no-gradient-accumulation-fusion " if IS_ROCM else "") | |
| + "--attention-softmax-in-fp32 " | |
| f"{'--no-gradient-accumulation-fusion ' if IS_ROCM else ''}" | |
| "--attention-softmax-in-fp32 " |
Depends on #1153.
hipBLASLt on gfx950 has no algorithm for TE's bias-fused wgrad GEMM (bf16→fp32 + BGRADB + accumulate). This conditionally disables gradient_accumulation_fusion and relaxes CI numerical checkers on ROCm. CUDA is unaffected.
Supersedes #1159