KEMBAR78
Wrap torch_python with torch_compile_options by cyyever · Pull Request #136743 · pytorch/pytorch · GitHub
Skip to content

Wrap torch_python with torch_compile_options #136743

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Wrap torch_python with torch_compile_options #136743

wants to merge 2 commits into from

Conversation

@cyyever
Copy link
Collaborator

@cyyever cyyever commented Sep 26, 2024

Fixes #ISSUE_NUMBER

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 26, 2024 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/136743

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 1ed5552 with merge base d558c1a (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Sep 26, 2024
@cyyever cyyever marked this pull request as draft September 26, 2024 08:02
@cyyever
Copy link
Collaborator Author

cyyever commented Oct 19, 2024

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased torch_python_option onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout torch_python_option && git pull --rebase)

@cyyever
Copy link
Collaborator Author

cyyever commented Oct 26, 2024

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased torch_python_option onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout torch_python_option && git pull --rebase)

@cyyever cyyever force-pushed the torch_python_option branch from 14595cf to 22edeeb Compare October 28, 2024 00:12
@cyyever
Copy link
Collaborator Author

cyyever commented Nov 6, 2024

@pytorchbot rebase -b main

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased torch_python_option onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout torch_python_option && git pull --rebase)

@cyyever cyyever added the ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR label Nov 6, 2024
@cyyever cyyever marked this pull request as ready for review November 6, 2024 11:05
@cyyever cyyever requested review from ezyang and malfet November 6, 2024 11:05
@cyyever cyyever force-pushed the torch_python_option branch from ca30607 to 011c52e Compare November 6, 2024 13:45
@cyyever cyyever force-pushed the torch_python_option branch from 7418fdb to 1ed5552 Compare November 6, 2024 15:25
ezyang
ezyang reviewed Nov 7, 2024
View reviewed changes
torch/CMakeLists.txt
torch_compile_options(torch_python) # see cmake/public/utils.cmake
if(NOT WIN32)
target_compile_options(torch_python PRIVATE
$<$<COMPILE_LANGUAGE:CXX>: -fvisibility=default>)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's going on here? Private visibility on torch_python seems doable no?

Copy link
Collaborator Author

@cyyever cyyever Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not feasible, because almost all types in torch_python were previously exported. It takes lots of efforts to identify and mark exported types with TORCH_PYTHON_API while still having the risk for the missing ones to break extensions. The safest way is to keep the old behavior and change gradually. In theory, it's doable, but in practice, we have to disable it for now.

@cyyever cyyever requested a review from ezyang November 7, 2024 15:54
@ezyang
Copy link
Contributor

ezyang commented Nov 7, 2024

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 7, 2024
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@cyyever cyyever deleted the torch_python_option branch December 2, 2024 04:38
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
@cyyever
Wrap torch_python with torch_compile_options (pytorch#136743)
ec91bda 
Fixes #ISSUE_NUMBER

Pull Request resolved: pytorch#136743
Approved by: https://github.com/ezyang
@cyyever cyyever mentioned this pull request Dec 11, 2024
Closed
@wschin wschin mentioned this pull request Feb 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezyang ezyang ezyang approved these changes

@malfet malfet Awaiting requested review from malfet

Assignees

No one assigned

Labels

ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR ciflow/trunk Trigger trunk jobs on your pull request Merged open source topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cyyever @pytorchmergebot @ezyang @pytorchbot