-
Notifications
You must be signed in to change notification settings - Fork 16
[DO NOT MERGE] Port changes from upstream PyTorch #73
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
base: master
Are you sure you want to change the base?
Conversation
| help="Source files to be excluded for hipify", | ||
| required=False) | ||
|
|
||
| parser.add_argument( |
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.
This PR removes some internal changes related to CuPy. Please don't remove it. This will impact other frameworks.
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.
Thanks for reviewing, @lcskrishna. I agree we should keep these changes. @liligwu Any reason to change hipify_cli.py? It doesn't even exist in PyTorch upstream, so it isn't about conflicts...
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.
Thanks for reviewing, @lcskrishna. I agree we should keep these changes. @liligwu Any reason to change hipify_cli.py? It doesn't even exist in PyTorch upstream, so it isn't about conflicts...
Yes, this file doesn't exist in Pytorch's hipify. but when I run the hipify_torch in FBGEMM, it reports "custom_map_list is not an argument ...", something like that. I have to remove all the statements related to custom_map_list, I think it's because of the API change in hipify_python.hipify()
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.
@liligwu Interesting, just saw the codebase of fbgemm looks like its using CMake API. There were some changes after we added custom map in CMake way. I would recommend rebase your code with latest hipify_torch rather than removing it altogether. This might impact xformers as well.
cc: @jithunnair-amd
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.
I am working on modifying the changes in this PR to reinstate some of the features we had added to hipify_torch. The custom_map_list was one of those features and it involved an addition of a keyword argument to hipify. It'll be part of the changes I'll reinstate
|
@jithunnair-amd You might have to add changes manually into this repo to sync with PT upstream as it contains some updates into this repo which are not updated to PT upstream. |
...due to pytorch/pytorch#137157
NOTE: This PR needs to be on-hold until the upstream hipify changes are re-merged, as they have been reverted in pytorch/pytorch@7e8dace since they broke Meta internal builds.