-
Notifications
You must be signed in to change notification settings - Fork 360
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
Nccl ops correction changes #3387
base: main
Are you sure you want to change the base?
Conversation
# transpose key deleted since not desirable to lower it to permute | ||
to_delete = { | ||
key | ||
for key in settings_aot_autograd["decompositions"] | ||
if "detach" in key._name or "transpose" in key._name | ||
} | ||
|
||
for key in to_delete: | ||
del settings_aot_autograd["decompositions"][key] |
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.
we have a remove_detach
lowering pass. Can that help here ?
if aten.detach in settings_aot_autograd["decompositions"]: | ||
del settings_aot_autograd["decompositions"][aten.detach] | ||
# transpose key deleted since not desirable to lower it to permute | ||
to_delete = { |
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.
Will this apply to all cases not just NCCL?
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.
You mean in the non distributed example? I am not sure about that answer, I added this for the llama3 example since I was issues in the model lowering and it was generating graph breaks at the wrong part, leading to complex input error. It can be added to all cases in case if we want to not lower transpose to permute.
No description provided.