Move MemoryFormat/Layout to headeronly#168034
Closed
mikaylagawarecki wants to merge 5 commits intogh/mikaylagawarecki/386/basefrom
Closed
Move MemoryFormat/Layout to headeronly#168034mikaylagawarecki wants to merge 5 commits intogh/mikaylagawarecki/386/basefrom
mikaylagawarecki wants to merge 5 commits intogh/mikaylagawarecki/386/basefrom
Conversation
[ghstack-poisoned]
This was referenced Nov 17, 2025
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/168034
Note: Links to docs will display an error until the docs builds have been completed. ⏳ 1 Pending, 1 Unrelated FailureAs of commit 32730de with merge base 1c04a43 ( UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
[ghstack-poisoned]
[ghstack-poisoned]
| // Specialization for c10::Layout => StableIValue | ||
| // Note that we call into the shim to translate between the user's | ||
| // Layout and libtorch's Layout, which can be different! | ||
| using c10::Layout; |
Contributor
Author
There was a problem hiding this comment.
for these two I don't error in the base case bcos per "BC" they should use the base case in 2.9 (wdyt of that haha)
torch/headeronly/core/Layout.h
Outdated
Comment on lines
51
to
53
| // Note: We can't use TORCH_CHECK here as it's not header-only | ||
| // Callers should ensure valid layout values | ||
| return stream << "Unknown"; |
Contributor
Author
There was a problem hiding this comment.
I'll fix this and below
Contributor
There was a problem hiding this comment.
Should be okay to just replace with STD_TORCH_CHECK
Contributor
There was a problem hiding this comment.
Actually....since that changes semantics...maybe an alternative is to just not migrate the >> functions
This was referenced Nov 19, 2025
janeyx99
approved these changes
Nov 19, 2025
torch/headeronly/core/Layout.h
Outdated
Comment on lines
51
to
53
| // Note: We can't use TORCH_CHECK here as it's not header-only | ||
| // Callers should ensure valid layout values | ||
| return stream << "Unknown"; |
Contributor
There was a problem hiding this comment.
Should be okay to just replace with STD_TORCH_CHECK
janeyx99
changed the title
…HECK" [ghstack-poisoned]
janeyx99
changed the title
janeyx99
changed the title
~This PR does change the semantics of the >> operator by using STD_TORCH_CHECK to throw the error instead of TORCH_CHECK. Jane (who is writing this message) thinks it is okay because it is the error case when an invalid MemoryFormat or Layout is getting passed into >>, so the UX benefits of TORCH_CHECK over STD_TORCH_CHECK there are not significant enough to warrant making a new copy of Layout and MemoryFormat's >> APIs.~ Never mind! We shouldn't change TORCH_CHECK to STD_TORCH_CHECK for core usage ever, cuz the traceback info and c10::Error is very much desired!! So the solution is to not migrate the >>s. I pushed new commits to the stack to remove the >> code, but for reference, 8a30179 has all the code that I ended up deleting. [ghstack-poisoned]
janeyx99
added
release notes: cpp
Contributor
|
@pytorchbot merge |
Collaborator
Merge startedYour 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 |
Collaborator
Merge failedReason: 1 jobs have failed, first few of them are: trunk / libtorch-linux-jammy-cuda12.8-py3.10-gcc11-debug / build Details for Dev Infra teamRaised by workflow job |
Contributor
|
@pytorchbot merge -i |
Collaborator
Merge startedYour change will be merged while ignoring the following 1 checks: trunk / libtorch-linux-jammy-cuda12.8-py3.10-gcc11-debug / build Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
JacobSzwejbka
pushed a commit
that referenced
this pull request
Dec 8, 2025
~This PR does change the semantics of the >> operator by using STD_TORCH_CHECK to throw the error instead of TORCH_CHECK. Jane (who is writing this message) thinks it is okay because it is the error case when an invalid MemoryFormat or Layout is getting passed into >>, so the UX benefits of TORCH_CHECK over STD_TORCH_CHECK there are not significant enough to warrant making a new copy of Layout and MemoryFormat's >> APIs.~ Never mind! We shouldn't change TORCH_CHECK to STD_TORCH_CHECK for core usage ever, cuz the traceback info and c10::Error is very much desired!! So the solution is to not migrate the >>s. I pushed new commits to the stack to remove the >> code, but for reference, 8a30179 has all the code that I ended up deleting. Pull Request resolved: #168034 Approved by: https://github.com/janeyx99 ghstack dependencies: #168025, #167802, #167803, #167804, #167962 Co-authored-by: Jane Xu <janeyx@meta.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR does change the semantics of the >> operator by using STD_TORCH_CHECK to throw the error instead of TORCH_CHECK. Jane (who is writing this message) thinks it is okay because it is the error case when an invalid MemoryFormat or Layout is getting passed into >>, so the UX benefits of TORCH_CHECK over STD_TORCH_CHECK there are not significant enough to warrant making a new copy of Layout and MemoryFormat's >> APIs.Never mind! We shouldn't change TORCH_CHECK to STD_TORCH_CHECK for core usage ever, cuz the traceback info and c10::Error is very much desired!! So the solution is to not migrate the >>s. I pushed new commits to the stack to remove the >> code, but for reference, 8a30179 has all the code that I ended up deleting.
Stack from ghstack (oldest at bottom):