[release/1.8.0] Added AITER as a submodule and use in fused_rope.py #222
Merged
amd-sriram merged 10 commits intomasterfrom Jul 8, 2025
Merged
[release/1.8.0] Added AITER as a submodule and use in fused_rope.py #222amd-sriram merged 10 commits intomasterfrom
amd-sriram merged 10 commits intomasterfrom
Conversation
…d rope test, reduced tolerances according to unit test in aiter repo.
7da8bac to
1f30182
Compare
…r backend if it is rocm and aiter is installed
…y error - ValueError: numpy.dtype size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject
alugorey
reviewed
Jun 5, 2025
alugorey
reviewed
Jun 5, 2025
|
Just a couple questions. |
xinyazhang
requested changes
Jun 5, 2025
pruthvistony
requested changes
Jun 6, 2025
…nd use pip install -e . instead of python setup.py develop for installing aiter.
…nc and select apex or aiter subclass based on AITER_ROPE_BACKEND value. The user can specify the environment variable USE_ROCM_AITER_ROPE_BACKEND to select between aiter and apex backends for fused rope.
…est otherwise use the original precision 1e-3
Collaborator
Author
|
@xinyazhang @alugorey @pruthvistony I have now made the requested changes. |
xinyazhang
reviewed
Jun 30, 2025
xinyazhang
left a comment
There was a problem hiding this comment.
Mostly cosmetic problems.
Also please go through the file to remove unnecessary changes.
remove spaces
pruthvistony
previously approved these changes
Jul 8, 2025
pruthvistony
left a comment
There was a problem hiding this comment.
LGTM. testing is good.
Wait for Andy and Xinya approval.
xinyazhang
previously approved these changes
Jul 8, 2025
xinyazhang
left a comment
There was a problem hiding this comment.
LGTM. The only concern is the pip install -e . for aiter.
Collaborator
Author
|
Currently there is a build issue related to aiter. I am trying to contact aiter team regarding. I will merge after the build issue is resolved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added AITER support in fused_rope.py for all 4 variants. Updated fused rope test, reduced tolerances according to unit test in aiter repo.
Tested UT - python tests/L0/run_transformer/test_fused_rope.py
Added aiter as a submodule and build it in setup.py if it is rocm.
For rocm, it uses AITER backend
For cuda, it uses apex native kernels
Tested with 6.5_internal_testing and upstream main
Fixes : https://ontrack-internal.amd.com/browse/SWDEV-496182