feat: detect when the resolved torch/CUDA ships no kernels for the local GPU#3104
feat: detect when the resolved torch/CUDA ships no kernels for the local GPU#3104golithe wants to merge 4 commits into
Conversation
anish-sahoo
left a comment
There was a problem hiding this comment.
Thanks for putting this together. This is a useful diagnostic and the overall approach looks promising. I found several compatibility edge cases, along with some test and documentation gaps, that need additional fixes before we can work toward merging this. Details are inline.
|
@anish-sahoo ty for your thorough review! i addressed your comments, PTAL |
anish-sahoo
left a comment
There was a problem hiding this comment.
Thanks for addressing the first review round. The mixed-GPU evaluation, exact-pin handling for ordinary torch==... requirements, local-tag boundary case, deterministic probe tests, docs, and rename all look good to me.
I found two remaining issues that need to be addressed before merge:
-
gpu_compatibility.gostrips the local wheel tag and then relies only onbuild.cuda. This can silence a real Blackwell incompatibility:torch==2.7.0+cu118with an explicitcuda: "12.8"passes the check because it compares the stripped2.7.0and base CUDA 12.8 against the floor. But Cog retains the+cu118wheel, and that wheel has no Blackwell kernels regardless of its CUDA base image. Please preserve/derive the wheel CUDA tag for compatibility evaluation, rather than losing it when normalizing the torch release, and add a fullConfig.Complete -> Checkregression test for this override combination. -
Config.TorchExactVersion()comparesrequirements.PackageName(pkg)directly to lowercase"torch". Exact valid requirements such astorch[extra]==2.4.1andTorch==2.4.1therefore return no torch pin and skip the GPU check. Please normalize the base distribution name (strip extras and normalize casing, ideally PEP 503-style) before matching, with an end-to-end check regression test.
good catch. now derives the wheel's CUDA from the
fixed. added |
|
@anish-sahoo adressed both, PTAL |
| // ships no Blackwell kernels even under cuda: "12.8", so evaluate against 11.8, not 12.8. | ||
| // Fall back to the build CUDA only when there is no cuXYZ tag to derive from. | ||
| effectiveCUDA := cuda | ||
| if wheelCUDA, _ := config.CUDAVersionFromTorchLocalTag(torchVersion); wheelCUDA != "" { |
There was a problem hiding this comment.
I re-tested this and need to correct my earlier assumption. The GPU check and the build resolve CUDA differently:
torch==2.7.0+cu118withcuda: "12.8"builds with the cu128 wheel, but the check uses cu118 and warns incorrectly.- An explicit cu118 index builds with the cu118 wheel, but the check uses
cuda: "12.8"and does not warn.
Could the check use the final resolved requirement and index from PythonRequirementsForArch? That would ensure it checks the wheel Cog will actually install.
There was a problem hiding this comment.
good catch again, and you're right.
- the check now resolves the wheel via the same path as
PythonRequirementsForArch(newConfig.ResolvedTorchWheel) and derives CUDA from the resolved package index instead of the requirement's local tag, so it matches what Cog installs in both directions:torch==2.7.0+cu118+cuda: "12.8"resolves to cu128 and stays silent, and an explicit--extra-index-url=.../cu118fires on 11.8. - Regressions for both in
TestResolvedTorchWheeland the twoGPUCompatibilityChecktests. PTAL.
torch==2.4.1builds green, and every kernel launch fails withCUDA error: no kernel image is available for execution on the devicetorch.cuda.is_available()returnsTrue, and PyTorch's own diagnostic is aUserWarningcog rundoes not work with A100 #389 is the same failure class on an A100 in 2022 (closed when the reporter worked around it); Blackwell makes it current againThis PR adds:
GPUCompatibilityChecktocog doctor, modelled onDockerChecknvidia-smi --query-gpu=compute_cap(every distinct GPU, not just the weakest, since a mixed-GPU host must clear each device's floor) and compares the resolved torch/CUDA against the oldest release known to ship kernels for it.cog doctoroutput when it fires:Notes for review:
torch._C._cuda_getArchFlags(), which works without a GPUPythonRequirementsForArch), not the raw requirement, so it's correct in both directions:torch==2.7.0+cu118withcuda: "12.8"resolves to thecu128wheel and stays silent, while an explicit--extra-index-url=.../cu118installs the cu118 wheel regardless ofcuda:and warns against 11.8SeverityWarning, matchingPythonVersionCheck: the image is valid and runs fine on other hardware; it only fails when executed on this machine's GPUgpu: false;COG_SKIP_GPU_CHECK=1skips it when building for different hardware than the local card. Doctor-only, no build-path changesTorch==andtorch[extra]==resolve rather than silently skipping the check.Testing:
evaluateGPUCompat), table-tested without a GPU, including+cu128local-tag pins and the new-torch/old-CUDA casetorch==2.4.1, silent on2.7.1and ongpu: falseOut of scope (deliberately):
MinimumTorchVersionand friends inpkg/dockerfile/base.go)tools/to regenerate them on demand could be a follow-up; fullcompatgenintegration seems like a poor fit, since new capability majors ship every couple of years and the probe needs multi-GB wheel installs. Happy to add the script here or in a follow-up PR if useful