[Fix] Make DeepGEMM triton fallback more robust#47126
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
a608a3f to
f8633dc
Compare
| return f"DeepGEMM requires {arch}; current device is SM{major}{minor}." | ||
|
|
||
| # Per the DeepGEMM README: SM90 needs CUDA 12.3+, SM100 needs CUDA 12.9+. | ||
| cuda_major, cuda_minor = get_cuda_runtime_version() |
There was a problem hiding this comment.
do you know why this doesn't catch the cuda version mismatch ? maybe we can make get_cuda_runtime_version respect CUDA_HOME if it's not the case already
There was a problem hiding this comment.
CUDA_HOME is user set / can not match between PATH and the env, this is an env-only check (torch does not use CUDA_HOME to check the version)
In any case it's nore about needing the CUDA toolkit at runtime: the best way to verify that is to actually know if it can be ran. As long as the result of load_deepgemm.... is cached it will only happen once
| f"DeepGEMM failed to JIT-compile a probe kernel, which usually means a broken or version-mismatched " | ||
| "CUDA toolchain. Check that `CUDA_HOME` points to a complete and version-consistent CUDA toolkit. You " | ||
| f"can re-run with `DG_JIT_DEBUG=1` for the compiler output. Original error: {type(e).__name__}: {e}" | ||
| ) |
There was a problem hiding this comment.
this however i'm very reluctant about, we should in theory be able to know whether a cuda toolkit is reachable and its version programmatically without having to allocate or run anything, wdyt ?
There was a problem hiding this comment.
Not sure about this really -- it depends on a few variables, and if the goal of the function is to load callables that will be JIT compiled, then it seems logical to check if the JIT compiling works. WDYT? Otherwise pretty hard to tell if a toolkit is working
There was a problem hiding this comment.
If this became a load time only error for you cuz tookits where not available,we should indeed do it!
There was a problem hiding this comment.
with the checks merged from #47154 the kernel will only be loaded when the env has the toolkit with nvcc, and nvcc claims to be of compatible with the kernels.
the real problem imo is that we used the wrong way to check the cuda versions (whichever libcudart loaded first), now we do the same as what deepgemm does internally (check cuda home, check nvcc, check their versions) so it should never be a problem unless the user manually created an environment with broken (cuda home + nvcc), in which case imo they should be told that their env is broken loudly
There was a problem hiding this comment.
not against the final compilation check in itself, but rather against downloading the full kernel and only realizing that its not compatible with your system after a couple minutes of rate limiting.
IlyasMoutawwakil
left a comment
There was a problem hiding this comment.
Thanks for catching this ! we do have a flag to disable the deepgemm fp8 linear, we should probably mention it in the error message in finegrained_fp8's linear dispatch.
f8633dc to
7d9d23b
Compare
Not sure what you mean here, can you add a permalink? I feel like this is ready for a second round of review / approval, thanks! |
|
I have opened #47154 based and into this branch, tell me what you think. |
| f"DeepGEMM failed to JIT-compile a probe kernel, which usually means a broken or version-mismatched " | ||
| "CUDA toolchain. Check that `CUDA_HOME` points to a complete and version-consistent CUDA toolkit. You " | ||
| f"can re-run with `DG_JIT_DEBUG=1` for the compiler output. Original error: {type(e).__name__}: {e}" | ||
| ) |
There was a problem hiding this comment.
If this became a load time only error for you cuz tookits where not available,we should indeed do it!
IlyasMoutawwakil
left a comment
There was a problem hiding this comment.
LGTM ! thanks for making it more robust 🙏
* Gate DeepGEMM JIT on the CUDA toolkit (nvcc/NVRTC), not the runtime DeepGEMM JIT-compiles its kernels with the system CUDA toolkit, so gating on the CUDA runtime version was the wrong signal (and on multi-toolkit boxes could read a stray/mismatched libcudart). Resolve the toolkit the way DeepGEMM does (respecting CUDA_HOME) and validate it statically before loading the kernel. - import_utils: add get_cuda_home + get_nvcc_version (read the toolkit version from version.json/version.txt/cuda.h, no subprocess); deprecate get_cuda_runtime_version. - deepgemm: gate on the resolved nvcc toolkit; when nvcc is missing/too old but torch's bundled libnvrtc is new enough, fall back to the NVRTC backend instead of failing. - test_modeling_common: mirror the gate (Hopper -> CUDA 12.3, Blackwell -> 12.9). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * TRANSFORMERS_DISABLE_DEEPGEMM_LINEAR in worst case scenario * remove nvrtc path as not usable with kernel hub build * trim comment * address comments * update comment --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
deb5b95 to
3c650e6
Compare
CI recapDashboard: View test results in Grafana |
This PR makes the triton fallback to DeepGEMM more robust:
CUDA_HOMEis not set, the current behavior is to fail with anAssertionErrorwhich is not caught by thetryblock because it specializes onImportError. To avoid this, we callget_mk_alignment()before creating theDeepGemmobject in atryblock and pass the missingCUDA_HOMEerror as anImportError-> triton fallback worksCUDA_HOMEis set but does not match the CUDA toolkit, the JIT compilation will fail but this is not caught at loading time, hence the problem manifests at Runtime and we hard-error instead of falling back to triton. To avoid this, we run a tiny JIT compiled kernel at load time (transform_sf_into_required_layout) and fallback to triton if the JIT compilation fails.Also, I think the
functools.cachedecorator never worked as intended because it does not catch error raised in the function, so any fail during_load_deepgemm_kernelwas bound to happen all over again. To avoid this, instead of raising the expectedImportErrorin the cached function, we wrap it into a function that will raise by itself.Result: on a machine with no
CUDA_HOMEor a mismatchingCUDA_HOME, we fall back to triton instead of erroring out. I have not tested on a machine with DeepGEMM but it should not change the behavior or a succesful load. Perhaps @IlyasMoutawwakil can confirm please? Thx!