Do not let one op builder's compatibility probe break importing deepspeed - #8216
Do not let one op builder's compatibility probe break importing deepspeed#8216adityasingh2400 wants to merge 4 commits into
Conversation
…peed Importing deepspeed probes every op for build compatibility, whether or not the caller will ever build that op. Nothing catches a probe that fails, so on a machine with a visible GPU but no CUDA toolkit the CUDA op builders raise MissingCUDAException out of is_compatible() and import deepspeed fails outright, even for a sharding only workload that needs no custom ops. ds_report fails the same way. This is the nvcc assumption reported in deepspeedai#7452. Add probe_is_compatible, which reports an op whose probe fails as not compatible and prints why, and use it for the two places that scan every op: the import time scan in git_version_info and the ds_report table. Also make installed_cuda_version raise MissingCUDAException when nvcc cannot be run. CUDA_HOME regularly points at a runtime only install with no nvcc under bin, and the raw FileNotFoundError from that case slipped past the two except MissingCUDAException handlers in builder.py that already fall back to a CPU only build. Fixes deepspeedai#7452 Signed-off-by: Aditya Singh <adisin650@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 866c2c96ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@adityasingh2400 thanks for the PR. How did this issue come up? |
Never mind, I just noticed the linked issue. Thanks! |
|
The The run ends with Nothing in this change should move memory use either, it only wraps an op builder's Thanks for merging master in. A re-run is probably all it needs, but shout if you would rather I did something on my side. |
3 participants
Fixes #7452
Importing deepspeed probes every op for build compatibility in git_version_info.py, whether or not the caller will ever build that op. Nothing catches a probe that fails, so on a machine with a visible GPU but no CUDA toolkit the CUDA op builders raise MissingCUDAException out of is_compatible() and import deepspeed fails outright, even for a sharding only workload that needs no custom ops. ds_report fails the same way. That is the nvcc assumption reported in the issue.
This adds probe_is_compatible, which reports an op whose probe fails as not compatible and prints why, and uses it in the two places that scan every op: the import time scan and the ds_report table.
It also makes installed_cuda_version raise MissingCUDAException when nvcc cannot be run. CUDA_HOME regularly points at a runtime only install with no nvcc under bin, and the raw FileNotFoundError from that case slipped past the two except MissingCUDAException handlers already in builder.py that fall back to a CPU only build.
Verification: added three tests to tests/unit/ops/test_op_builder.py covering a runtime only CUDA_HOME, a probe that raises, and a probe that answers. All 16 tests in that file pass, with the pre-existing CUDA fork test skipped for lack of a GPU. The new tests fail against the unmodified code, and I confirmed separately that installed_cuda_version raised FileNotFoundError rather than MissingCUDAException before the change. yapf and flake8 are clean on the changed files.