KEMBAR78
Fix torch.load(..., weights_only=True) for NT by jbschlosser · Pull Request #112516 · pytorch/pytorch · GitHub
Skip to content

Fix torch.load(..., weights_only=True) for NT #112516

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

Fix torch.load(..., weights_only=True) for NT #112516

wants to merge 1 commit into from

Conversation

jbschlosser
Copy link
Contributor

@jbschlosser jbschlosser commented Oct 31, 2023 •
edited
Loading

Stack from ghstack (oldest at bottom):

Found when looking into #112509

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 31, 2023 •
edited
Loading

🔗 Helpful Links

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

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 f890557 with merge base 86196bf (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

jbschlosser added a commit that referenced this pull request Oct 31, 2023
@jbschlosser
Fix torch.load(..., weights_only=True) for NT
4b93a58 
ghstack-source-id: c23cb40
Pull Request resolved: #112516
@jbschlosser jbschlosser added topic: bug fixes topic category release notes: nested tensor Changes that have a direct impact on nested tensors topic: improvements topic category and removed topic: bug fixes topic category labels Oct 31, 2023
@jbschlosser
Copy link
Contributor Author

@pytorchbot merge

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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / macos-12-py3-arm64-mps / test (mps, 1, 1, macos-m1-12)

Details for Dev Infra team Raised by workflow job

albanD
albanD reviewed Oct 31, 2023
View reviewed changes
torch/_weights_only_unpickler.py
torch._utils._rebuild_tensor_v2,
torch._utils._rebuild_sparse_tensor,
torch._utils._rebuild_meta_tensor_no_storage,
torch._utils._rebuild_nested_tensor,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not add torch._nested_view_from_buffer here directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that initially but it didn't work? It was getting caught up on the built-in getattr

Copy link
Collaborator

Choose a reason for hiding this comment

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

The one for the torch module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I believe that's the one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any ideas on how to solve this?

@jbschlosser
Copy link
Contributor Author

@pytorchbot merge -f "ignore spurious failure"

@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

@facebook-github-bot facebook-github-bot deleted the gh/jbschlosser/95/head branch November 5, 2023 15:25
xuhancn pushed a commit to xuhancn/pytorch that referenced this pull request Nov 7, 2023
@jbschlosser @xuhancn
Fix torch.load(..., weights_only=True) for NT (pytorch#112516)
0bdc982 
Found when looking into pytorch#112509
Pull Request resolved: pytorch#112516
Approved by: https://github.com/soulitzer
Skylion007 pushed a commit to Skylion007/pytorch that referenced this pull request Nov 14, 2023
@jbschlosser @Skylion007
Fix torch.load(..., weights_only=True) for NT (pytorch#112516)
2042c35 
Found when looking into pytorch#112509
Pull Request resolved: pytorch#112516
Approved by: https://github.com/soulitzer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albanD albanD albanD left review comments

@soulitzer soulitzer soulitzer approved these changes

@cpuhrsch cpuhrsch Awaiting requested review from cpuhrsch

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged release notes: nested tensor Changes that have a direct impact on nested tensors topic: improvements topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jbschlosser @pytorchmergebot @albanD @soulitzer