-
-
Notifications
You must be signed in to change notification settings - Fork 12.5k
Only register rocm_aiter_ops if aiter is found #28428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: mgoin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly addresses an issue where importing rocm_aiter_ops at the module level in fp8_utils.py caused unnecessary warnings on non-ROCm platforms. By moving the import into the _run_aiter method, it becomes a lazy import, ensuring that ROCm-specific code is only loaded when it is actually used. This change is clean, effective, and has no significant performance impact. The fix is well-implemented and resolves the reported problem.
yewentao256
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the work!
Signed-off-by: mgoin <[email protected]>
Signed-off-by: mgoin <[email protected]>
tjtanaa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for the fix.
Signed-off-by: mgoin <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
Signed-off-by: mgoin <[email protected]> (cherry picked from commit f2d9ad0)
Signed-off-by: mgoin <[email protected]> (cherry picked from commit f2d9ad0)
Signed-off-by: mgoin <[email protected]>
Signed-off-by: mgoin <[email protected]> Signed-off-by: Xingyu Liu <[email protected]>
Purpose
In #24490 there was an addition of a new import for rocm.py
Without this change, you can see the rocm import warnings on nvidia devices by default
Thanks to @tjtanaa, he pointed out there was an oversight and the ops class should act as a no-op when it is on non-ROCm platform and aiter is not installed.
I found the culprit using
Output:
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.