Implement a constructor for nested_tensor that is similar to torch.tensor() #88213
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/88213
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 76704c9: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
There was a problem hiding this comment.
Why are these changes needed?
There was a problem hiding this comment.
The compiler started complaining for some reason.
torch/csrc/utils/nested.cpp
Outdated
There was a problem hiding this comment.
Is it possible to rely on _nested_tensor_from_tensor_list for all the dtype, device, etc. checks?
There was a problem hiding this comment.
Looking at the implementation for both torch::tensor() and _nested_tensor_from_tensor_list, we do the same amount of checks
There was a problem hiding this comment.
Sure, but this could deduplicate code
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
ani300
force-pushed
the
export-D40878312
branch
from
2587a6b to
3eb62e7
Compare
ani300
force-pushed
the
export-D40878312
branch
from
3eb62e7 to
d6e8c1d
Compare
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
ani300
force-pushed
the
export-D40878312
branch
from
d6e8c1d to
a29a8ce
Compare
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
There was a problem hiding this comment.
nit: I don't think you should need this check
>>> torch.nested.nested_tensor([])
nested_tensor([
])
There was a problem hiding this comment.
this is for the C++ path, given that python now takes python vars it needs different impls (or at least torch::tensor does so)
ani300
force-pushed
the
export-D40878312
branch
from
f2663fb to
ad8093d
Compare
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
ani300
force-pushed
the
export-D40878312
branch
from
ad8093d to
22c0bd9
Compare
ani300
force-pushed
the
export-D40878312
branch
from
22c0bd9 to
63b2250
Compare
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
torch/nested/__init__.py
Outdated
There was a problem hiding this comment.
nit: when using _add_docstr, in order to render the arguments for the function a line containing the signature of the function needs to be added here (see to_padded_tensor above for an example)
(the preview for the docs is available under helpful links in the pytorch-bot comment on this PR if you would like to look at the rendering!)
ani300
force-pushed
the
export-D40878312
branch
from
63b2250 to
4270b59
Compare
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
ani300
force-pushed
the
export-D40878312
branch
from
4270b59 to
d7a8c00
Compare
…nsor() (pytorch#88213) Summary: Pull Request resolved: pytorch#88213 This diff merges both previous implementations of constructors for nested tensors, the one from lists of tensors and the one with arbitrary python lists, adn implements it in pytorch core so no extensions are needed to construct NT. Test Plan: All nested tensor tests pass successfully. Specifically: `buck test mode/dev-nosan //caffe2/test:nested` Reviewed By: cpuhrsch, mikekgfb Differential Revision: D40878312 fbshipit-source-id: e9f36fa66287299efc144c45b65a8f6cbe248ca3
|
This pull request was exported from Phabricator. Differential Revision: D40878312 |
ani300
force-pushed
the
export-D40878312
branch
from
d7a8c00 to
76704c9
Compare
|
@pytorchbot merge -g |
Merge failedReason: This PR has internal changes and must be landed via Phabricator Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -g |
Merge startedYour change will be merged once all checks on your PR pass since you used the green (-g) flag (ETA: 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
…nsor() (pytorch#88213) Summary: This diff merges both previous implementations of constructors for nested tensors, the one from lists of tensors and the one with arbitrary python lists, adn implements it in pytorch core so no extensions are needed to construct NT. Pull Request resolved: pytorch#88213 Approved by: https://github.com/cpuhrsch
5 participants
Summary: This diff merges both previous implementations of constructors for nested tensors, the one from lists of tensors and the one with arbitrary python lists, adn implements it in pytorch core so no extensions are needed to construct NT.