fix(ci): cudaRoundMode typing failure in FP8 test#834
Conversation
|
Thanks in advance - unable to reproduce the error locally so would have to use CI to test this. @gmarkall |
| E5M2 = 1 | ||
|
|
||
|
|
||
| class cudaRoundMode(IntEnum): |
There was a problem hiding this comment.
@gmarkall @isVoid what does it take for numba-cuda to understand cudaRoundMode from cuda-bindings as a legal enum, without numba-cuda having to repeat the definitions?
On the cuda-bindings side, we switched to a custom fast enum from the builtin IntEnum to reduce Python overhead (cc @mdboom), and we can evaluate if a patch on cuda-bindings makes better sense, or if we could just register the type in numba-cuda (or both).
There was a problem hiding this comment.
We'd need to add typing to Numba for the FastEnumMetaclass type for Numba to recognise it.
There was a problem hiding this comment.
Would it help if we add __int__ to the fast enum class?
There was a problem hiding this comment.
I don't think Numba-CUDA will see that. Apart from a small set of cases:
numba-cuda/numba_cuda/numba/cuda/typing/typeof.py
Lines 50 to 97 in 893dde7
Numba-CUDA looks at the Python type to map to the Numba type.
Maybe we could / should add recognition of __int__ as well, but it would need a Numba-CUDA change in addition to adding __int__ to the fast enum class.
There was a problem hiding this comment.
Ah, it's done by the same function I just touched recently... Got it. So this is a corner case where Python types do matter and duck-typing does not work.
I would suggest that we merge this PR as a workaround to unblock the CI, and discuss a long-term fix. (My 2c is if the surface area grows to other enums we should register the fast enum type, but one-off patches like this are not bad.) WDYT?
There was a problem hiding this comment.
Sorry I got around to this a bit late. I saw this error yesterday in my Numbast CI but didn't trace it to the bottom. The cuda_fp8.py file was auto generated by Numbast. If we decided on a fast enum type for cuda types we should probably inform Numbast to generate corresponding logics.
|
/ok to test cfc3787 |
To support the new `FastEnum` class in `cuda_bindings` 13.2, this adds new type registrations to support them. These instances are otherwise 100% API-compatible with `enum.IntEnum`, so there is no new logic. This should hopefully be a more sustainable solution than overriding individual enums, so this also reverts #834.
- bump version to 0.29.0 - fix: normalize numpy integer types to python int to prevent overflow errors (#774) - Support cuda.core.GraphBuilder as a kernel-launch stream (#836) - Support cuda_bindings FastEnum (#837) - fix(ci): cudaRoundMode typing failure in FP8 test (#834) - Use `cuda-python` for `nvvm` bindings (#818) - Fix mixed-IR liveness for inline overload DCE (#795) - Use dbg.declare for scalar kernel parameters (#828) - Fix FP8 uint64 cast flake on Windows (#829) - Extend dbg.value coverage to loadvar for scalar kernel parameters (#813) <!-- Thank you for contributing to numba-cuda :) Here are some guidelines to help the review process go smoothly. 1. Please write a description in this text box of the changes that are being made. 2. Please ensure that you have written units tests for the changes made/features added. 3. If you are closing an issue please use one of the automatic closing words as noted here: https://help.github.com/articles/closing-issues-using-keywords/ 4. If your pull request is not ready for review but you want to make use of the continuous integration testing facilities please label it with `[WIP]`. 5. If your pull request is ready to be reviewed without requiring additional work on top of it, then remove the `[WIP]` label (if present) and replace it with `[REVIEW]`. If assistance is required to complete the functionality, for example when the C/C++ code of a feature is complete but Python bindings are still required, then add the label `[HELP-REQ]` so that others can triage and assist. The additional changes then can be implemented on top of the same PR. If the assistance is done by members of the rapidsAI team, then no additional actions are required by the creator of the original PR for this, otherwise the original author of the PR needs to give permission to the person(s) assisting to commit to their personal fork of the project. If that doesn't happen then a new PR based on the code of the original PR can be opened by the person assisting, which then will be the PR that will be merged. 6. Once all work has been done and review has taken place please do not add features or make changes out of the scope of those requested by the reviewer (doing this just add delays as already reviewed code ends up having to be re-reviewed/it is hard to tell what is new etc!). Further, please do not rebase your branch on main/force push/rewrite history, doing any of these causes the context of any comments made by reviewers to be lost. If conflicts occur against main they should be resolved by merging main into the branch used for making the pull request. Many thanks in advance for your cooperation! --> Co-authored-by: Michael Wang <isVoid@users.noreply.github.com>
Attempt to fix this issue.
cuda-bindings
13.2.0changedcudaRoundModefrom a standard PythonIntEnumto aFastEnumMetaclasstype. Numba's type inference cannot resolve FastEnumMetaclass types, causing three FP8 tests to fail (ref).This PR replaces adds local IntEnum in
cuda_fp8.py, matching the pattern already used forsaturation_tand `fp8_interpretation_t.If this works, let file a issue on the Numbast side (if needed).