| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
TestCUDAForward.test_forward and TestCUDAForwardStochastic.test_forward_stochastic build and run the CUDA-only DeepSpeedTransformerLayer (TransformerBuilder), but unlike their sibling test_forward_with_small_bsz they lacked the TransformerBuilder compatibility guard. On cpu-torch-latest runners whose CPU advertises oneDNN fp16 support, the module-level fp16 skip is bypassed (CPU_Accelerator.supported_dtypes() then includes float16), so these tests run and fail trying to build a CUDA kernel on CPU. Add the same skipif guard so they are skipped when TransformerBuilder is not compatible. Signed-off-by: Olatunji Ruwase <tunji.ruwase@snowflake.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: f820289573
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Sorry, something went wrong.
| world_size = 1 | ||
| reuse_dist_env = True | ||
|
|
||
| @pytest.mark.skipif(not deepspeed.ops.__compatible_ops__[TransformerBuilder.NAME], |
There was a problem hiding this comment.
Add the required Signed-off-by trailer
This reviewed commit is a non-merge commit with one parent, but its commit message does not contain a Signed-off-by trailer. The repository requires every non-merge commit to be signed off, so this needs to be recreated or amended with --signoff before it can satisfy the project’s commit requirements.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
tests/unit/ops/accelerators/test_accelerator_forward.py has three transformer-kernel test methods, but only TestCUDAForwardSmallBatchSize.test_forward_with_small_bsz guards on TransformerBuilder compatibility. TestCUDAForward.test_forward and TestCUDAForwardStochastic.test_forward_stochastic do not.
These tests build and run the CUDA-only DeepSpeedTransformerLayer. The module has a top-level fp16 gate:
On CPU, CPU_Accelerator.supported_dtypes() includes float16 only when is_fp16_supported() is true, which resolves to torch.ops.mkldnn._is_mkldnn_fp16_supported() -- a runtime CPU-capability probe. GitHub's ubuntu-24.04 runners are heterogeneous:
This makes cpu-torch-latest flaky depending on which runner the job lands on.
Fix
Add the existing @pytest.mark.skipif(not deepspeed.ops.__compatible_ops__[TransformerBuilder.NAME], ...) guard (already used by test_forward_with_small_bsz) to test_forward and test_forward_stochastic, so they are skipped whenever the CUDA TransformerBuilder op is not compatible on the host.
Test plan
Made with Cursor