Skip to content

[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.

Sign up for GitHub

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

Draft
jithunnair-amd wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

[DO NOT MERGE] Port changes from upstream PyTorch #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52e1959
port from pytorch due to https://github.com/pytorch/pytorch/pull/137157
liligwu Oct 8, 2024
4812659
Update hipify_python.py
jithunnair-amd Oct 9, 2024
2e693df
restore permissions
jithunnair-amd Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions hipify_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ def main():
help="Source files to be excluded for hipify",
required=False)

parser.add_argument(
Copy link
Copy Markdown
Contributor

@lcskrishna lcskrishna Oct 9, 2024

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.

Copy link
Copy Markdown
Collaborator Author

@jithunnair-amd jithunnair-amd Oct 9, 2024

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...

Copy link
Copy Markdown
Contributor

@liligwu liligwu Oct 9, 2024

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()

Copy link
Copy Markdown
Contributor

@lcskrishna lcskrishna Oct 9, 2024

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

Copy link
Copy Markdown
Collaborator Author

@jithunnair-amd jithunnair-amd Oct 9, 2024

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

'--custom-map-json',
type=str,
help="path of json which contains project specific hipifying mappings",
required=False)

parser.add_argument(
'--dump-dict-file',
default='hipify_output_dict_dump.txt',
Expand Down Expand Up @@ -95,7 +89,6 @@ def main():
ignores = json_args['ignores']
else:
ignores = []
custom_map_list=json_args.get("custom_map_json", "")
if(json_args.get('extra_files') is not None):
extra_files = json_args['extra_files']
else:
Expand All @@ -120,7 +113,6 @@ def main():
else args.ignores.strip("[]").split(";")
header_include_dirs=args.header_include_dirs if type(args.header_include_dirs) is list \
else args.header_include_dirs.strip("[]").split(";")
custom_map_list=args.custom_map_json or ""
extra_files = []
hipify_extra_files_only = False
dump_dict_file = args.dump_dict_file
Expand All @@ -130,9 +122,7 @@ def main():
project_directory=project_directory,
output_directory=output_directory,
includes=includes,
ignores=ignores,
header_include_dirs=header_include_dirs,
custom_map_list=custom_map_list,
ignores=ignores, header_include_dirs=header_include_dirs,
extra_files=extra_files,
is_pytorch_extension=True,
hipify_extra_files_only=hipify_extra_files_only,
Expand Down
Loading