Skip to content

[PyTorch][AMD] fix hipify_python #76720

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

Closed

Conversation

shintaro-iwasaki
Copy link
Contributor

Summary:
This PR fixes an issue in hipify_python introduced by #76141.

#76141 made all the includes paths "absolute", but this was not done for args.extra_include_dir; new_dir, which is a relative path, is directly added to includes. This PR fixes it by passing the absolute path (abs_new_dir).

Test Plan: CI

Differential Revision: D36089556

Summary:
This PR fixes an issue in hipify_python introduced by pytorch#76141.

pytorch#76141 made all the `includes` paths "absolute", but this was not done for `args.extra_include_dir`; `new_dir`, which is a relative path, is directly added to `includes`. This PR fixes it by passing the absolute path (`abs_new_dir`).

Test Plan: CI

Differential Revision: D36089556

fbshipit-source-id: eb9c68b8d9a99dd5bdcf621ecdd6f18470f4321b
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 3, 2022

🔗 Helpful links

💊 CI failures summary and remediations

As of commit c9dfd24 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36089556

@shintaro-iwasaki
Copy link
Contributor Author

cc @rraminen, @jeffdaily I'd appreciate your feedback.

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works for me!

@shintaro-iwasaki
Copy link
Contributor Author

Thanks for your reviews, @jeffdaily, @albanD, abd @osalpekar. I will merge it.

@rraminen
Copy link
Contributor

rraminen commented May 3, 2022

LGTM

facebook-github-bot pushed a commit that referenced this pull request May 3, 2022
Summary:
Pull Request resolved: #76720

This PR fixes an issue in hipify_python introduced by #76141.

#76141 made all the `includes` paths "absolute", but this was not done for `args.extra_include_dir`; `new_dir`, which is a relative path, is directly added to `includes`. This PR fixes it by passing the absolute path (`abs_new_dir`).

Test Plan: CI

Reviewed By: albanD

Differential Revision: D36089556

fbshipit-source-id: 1607075a4cb13696c1b25923f56b08a8cb3c6578
@shintaro-iwasaki shintaro-iwasaki deleted the export-D36089556 branch May 3, 2022 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants