Skip to content

replace torch::make_unique with std::make_unique #108866

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 1 commit into from
Closed

replace torch::make_unique with std::make_unique #108866

wants to merge 1 commit into from

Conversation

cyyever
Copy link
Collaborator

@cyyever cyyever commented Sep 8, 2023 •
edited
Loading

It should be safe to remove the old torch::make_unique functions.

cc @huydhn

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 8, 2023 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit ab2fcee with merge base 94a54b8 (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 release notes: cpp release notes category label Sep 8, 2023
@cyyever cyyever force-pushed the std_make_unique branch 3 times, most recently from 0b61390 to ac9dcc3 Compare September 8, 2023 15:14
@cyyever
Copy link
Collaborator Author

cyyever commented Sep 8, 2023

@pytorchbot label ciflow/binaries

@pytorch-bot pytorch-bot bot added the ciflow/binaries Trigger all binary build and upload jobs on the PR label Sep 8, 2023
@cyyever cyyever force-pushed the std_make_unique branch 4 times, most recently from 9029a02 to 63c9344 Compare September 8, 2023 16:43
@cyyever cyyever changed the title use std::make_unique replace torch::make_unique with std::make_unique Sep 8, 2023
@drisspg drisspg added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 11, 2023
albanD
albanD approved these changes Sep 14, 2023
View reviewed changes
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.

Nice cleanup! Thanks!

@cyyever
Copy link
Collaborator Author

cyyever commented Sep 14, 2023

@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 std_make_unique onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout std_make_unique && git pull --rebase)

@cyyever
Copy link
Collaborator Author

cyyever commented Sep 14, 2023

@pytorchmergebot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 14, 2023
@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

@facebook-github-bot
Copy link
Contributor

@huydhn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@huydhn
Copy link
Contributor

huydhn commented Sep 14, 2023 •
edited
Loading

It looks like torch::make_unique is still used internally. Let me see if their numbers are few enough so they can be fixed to unblock this change

Updated: Found only one instance of torch::make_unique internally, so I have prepared a fix to clean it up.

@cyyever cyyever deleted the std_make_unique branch September 15, 2023 00:00
@clee2000
Copy link
Contributor

@pytorchbot revert -m "Sorry but I found more usages of torch::make_unique internally, I can go change all of these, but I'd prefer if that gets done before this gets merged" -c ghfirst

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

@cyyever your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Sep 17, 2023
@pytorchmergebot
Revert "replace torch::make_unique with std::make_unique (#108866)"
525e4f4 
This reverts commit 03e35ef.

Reverted #108866 on behalf of https://github.com/clee2000 due to Sorry but I found more usages of `torch::make_unique` internally, I can go change all of these, but I'd prefer if that gets done before this gets merged ([comment](#108866 (comment)))
@cyyever cyyever restored the std_make_unique branch September 21, 2023 10:24
@cyyever cyyever mentioned this pull request Sep 21, 2023
Closed
pytorchmergebot pushed a commit that referenced this pull request Sep 21, 2023
@cyyever @pytorchmergebot
[Reland] Move torch::make_unique to std::make_unique (#109780)
e9e93c5 
We can first try to move torch::make_unique to std::make_unique despite reverting of #108866 .

Pull Request resolved: #109780
Approved by: https://github.com/ezyang
@cyyever cyyever deleted the std_make_unique branch February 13, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albanD albanD albanD approved these changes

@mrshenli mrshenli Awaiting requested review from mrshenli

@zhaojuanmao zhaojuanmao Awaiting requested review from zhaojuanmao

@rohan-varma rohan-varma Awaiting requested review from rohan-varma

@H-Huang H-Huang Awaiting requested review from H-Huang

@awgu awgu Awaiting requested review from awgu

@kwen2501 kwen2501 Awaiting requested review from kwen2501

@wanchaol wanchaol Awaiting requested review from wanchaol

@fegin fegin Awaiting requested review from fegin

@fduwjj fduwjj Awaiting requested review from fduwjj

@kiukchung kiukchung Awaiting requested review from kiukchung

@d4l3k d4l3k Awaiting requested review from d4l3k

@wz337 wz337 Awaiting requested review from wz337

@soulitzer soulitzer Awaiting requested review from soulitzer soulitzer is a code owner

Assignees

No one assigned

Labels

ciflow/binaries Trigger all binary build and upload jobs on the PR ciflow/trunk Trigger trunk jobs on your pull request Merged open source release notes: cpp release notes category Reverted triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@cyyever @pytorchmergebot @facebook-github-bot @huydhn @clee2000 @albanD @pytorchbot @drisspg