Pickle support for NT #110219
Pickle support for NT #110219
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/110219
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 2f9f7df with merge base 7c1702f ( 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. |
[ghstack-poisoned]
| "fill.Scalar", # only used by the functionalization pass | ||
| "lift.*", | ||
| "normal_functional", # only used by the functionalization pas | ||
| "_nested_view_from_buffer", # View only version of _nested_from_buffer. This will force users to only use the "safe" version. |
There was a problem hiding this comment.
What about using the "safe" version? :p
There was a problem hiding this comment.
I'm not entirely clear on this "safe" version comment (cc @drisspg). AFAIK this is the only way to construct an NT directly from its parts without copying, and _nested_from_buffer doesn't exist despite what the comment says :p
Imo we should either expose this to python or provide a proper public API for this functionality, and the latter would take design work that isn't relevant to our current focus: the subclass version of NT.
There was a problem hiding this comment.
Discussed offline with Driss: we're okay opening this private API up for now
| return (torch._utils._rebuild_sparse_tensor, args_sparse_compressed) | ||
| elif self.is_nested: | ||
| args_nested = ( | ||
| self.values(), |
There was a problem hiding this comment.
This won't work for arbitrary NestedTensors though right?
There was a problem hiding this comment.
Ok true, values() has a contiguity restriction :(
Edit: wait no it doesn't; it calls unsafe_storage_as_tensor() directly with no contiguity checks. It probably should be more restrictive, and we should have a private API for its current behavior.
There was a problem hiding this comment.
Discussed offline: eventually introduce a private API that does what values() does now and deprecate values() in its current form, to be replaced by appropriate usage of view(-1) / reshape(-1).
Fixes #104198 [ghstack-poisoned]
Fixes #104198 [ghstack-poisoned]
|
@pytorchbot merge |
Merge failedReason: This PR needs a If not, please add the To add a label, you can comment to pytorchbot, for example For more information, see Details for Dev Infra teamRaised by workflow job |
jbschlosser
added
topic: improvements
|
@pytorchbot merge |
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 |
Merge failedReason: 1 jobs have failed, first few of them are: linux-binary-libtorch-cxx11-abi / libtorch-cpu-shared-with-deps-cxx11-abi-test / test Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -f "ignore spurious failure" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Fixes #110161 Allows NTs to be used in DataLoaders with `num_workers > 1`. Pull Request resolved: #110292 Approved by: https://github.com/cpuhrsch ghstack dependencies: #110219
4 participants
Stack from ghstack (oldest at bottom):
Fixes #104198