Skip to content

Fix missing Torch includes #5118

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

Merged
merged 2 commits into from
Dec 22, 2021
Merged

Fix missing Torch includes #5118

merged 2 commits into from
Dec 22, 2021

Conversation

peterbell10
Copy link
Contributor

The torchvision build currently relies on transitive includes through unrelated torch headers which is causing build failures in pytorch/pytorch#68693.

@facebook-github-bot
Copy link

facebook-github-bot commented Dec 21, 2021

💊 CI failures summary and remediations

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


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

@datumbox
Copy link
Contributor

@peterbell10 Thanks for the PR.

We are intentionally trying to be more selective on our includes. Can you adapt the PR to explicitly import those relevant missed includes rather than just importing torch/library.h?

@peterbell10
Copy link
Contributor Author

peterbell10 commented Dec 21, 2021

TORCH_LIBRARY_IMPL/FRAGMENT is defined in torch/library.h so it doesn't get any more specific.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Fair enough, for TORCH_LIBRARY_IMPL. I got one more comment, let me know what you think.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@datumbox datumbox merged commit 8096c1b into pytorch:main Dec 22, 2021
@github-actions
Copy link

Hey @datumbox!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@peterbell10 peterbell10 deleted the fix-includes branch December 22, 2021 16:14
facebook-github-bot pushed a commit that referenced this pull request Dec 29, 2021
Summary:
* Fix missing Torch includes

* Import op_registration directly

Reviewed By: prabhat00155

Differential Revision: D33351108

fbshipit-source-id: a97f347898867c535433a28c44ca886f65292dcd
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.

3 participants