Skip to content
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

Expose Storage _use_count API in Python #139426

Open
wants to merge 3 commits into
base: gh/janeyx99/200/base
Choose a base branch
from

Conversation

janeyx99
Copy link
Contributor

@janeyx99 janeyx99 commented Oct 31, 2024

Would be nice to replace the torch._C._storage_Use_Count call in pytorch/torchtune#1936, at least without needing to know about _cdata in OSS code.

Initially keeping it private as Tensor._use_count is also private.

In favor over #139109 in solving the same problem, as exposing an existing API is better than adding a new one (and this enables a more robust fix)

Stack from ghstack (oldest at bottom):

Copy link

pytorch-bot bot commented Oct 31, 2024

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 3615247 with merge base 33dce10 (image):

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

janeyx99 added a commit that referenced this pull request Oct 31, 2024
ghstack-source-id: 54f77ee5f1fd28f67d1c7889a244be799e7a46e6
Pull Request resolved: #139426
@janeyx99 janeyx99 added the release notes: python_frontend python frontend release notes category label Oct 31, 2024
Needed by pytorch/torchtune#1936

In favor over #139109, as exposing an existing API is better than adding a new one (and this enables a more robust fix)




[ghstack-poisoned]
janeyx99 added a commit that referenced this pull request Nov 1, 2024
ghstack-source-id: 30622d3e144f4877ba46266ab5a80b88ffe6f071
Pull Request resolved: #139426
@janeyx99
Copy link
Contributor Author

janeyx99 commented Nov 1, 2024

@pytorchbot merge

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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 2 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

Needed by pytorch/torchtune#1936

In favor over #139109, as exposing an existing API is better than adding a new one (and this enables a more robust fix)




[ghstack-poisoned]
janeyx99 added a commit that referenced this pull request Nov 1, 2024
ghstack-source-id: e67fe62f07599847b7652cac0ca264cc3f2701d8
Pull Request resolved: #139426
@janeyx99 janeyx99 changed the title Expose Storage use_count API in Python Expose Storage _use_count API in Python Nov 1, 2024
@janeyx99
Copy link
Contributor Author

janeyx99 commented Nov 1, 2024

@pytorchbot merge

@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

@pytorchmergebot
Copy link
Collaborator

The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command
For more information see pytorch-bot wiki.

@janeyx99
Copy link
Contributor Author

janeyx99 commented Nov 2, 2024

@pytorchbot merge

@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / macos-py3-arm64 / build

Details for Dev Infra team Raised by workflow job

@janeyx99
Copy link
Contributor Author

janeyx99 commented Nov 2, 2024

@pytorchbot merge -f "broken trunk and mac build timeout don’t look like my fault"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

@pytorch pytorch deleted a comment from pytorch-bot bot Nov 3, 2024
@huydhn
Copy link
Contributor

huydhn commented Nov 3, 2024

@pytorchbot revert -m 'Sorry for reverting your change, but it is failing some inductor job in trunk' -c nosignal

test_torch.py::TestTorch::test_storage__use_count_APIs_align GH job link HUD commit link

@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

@janeyx99 your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Nov 3, 2024
This reverts commit e31136d.

Reverted #139426 on behalf of https://github.com/huydhn due to Sorry for reverting your change, but it is failing some inductor job in trunk ([comment](#139426 (comment)))
@pytorchmergebot pytorchmergebot added Reverted ci-no-td Do not run TD on this PR labels Nov 3, 2024
rahulsingh-intel pushed a commit to rahulsingh-intel/pytorch that referenced this pull request Nov 5, 2024
Would be nice to replace the torch._C._storage_Use_Count call in pytorch/torchtune#1936, at least without needing to know about _cdata in OSS code.

Initially keeping it private as Tensor._use_count is also private.

In favor over pytorch#139109 in solving the same problem, as exposing an existing API is better than adding a new one (and this enables a more robust fix)

Pull Request resolved: pytorch#139426
Approved by: https://github.com/soulitzer
rahulsingh-intel pushed a commit to rahulsingh-intel/pytorch that referenced this pull request Nov 5, 2024
This reverts commit e31136d.

Reverted pytorch#139426 on behalf of https://github.com/huydhn due to Sorry for reverting your change, but it is failing some inductor job in trunk ([comment](pytorch#139426 (comment)))
Copy link
Contributor

github-actions bot commented Jan 2, 2025

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-no-td Do not run TD on this PR ciflow/trunk Trigger trunk jobs on your pull request Merged release notes: python_frontend python frontend release notes category Reverted Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants