Skip to content
This repository has been archived by the owner on Nov 15, 2022. It is now read-only.

[DESIGN] as_nested_tensor doesnt behave same as as_tensor #150

Closed
izdeby opened this issue May 7, 2020 · 2 comments
Closed

[DESIGN] as_nested_tensor doesnt behave same as as_tensor #150

izdeby opened this issue May 7, 2020 · 2 comments
Labels

Comments

@izdeby
Copy link
Contributor

izdeby commented May 7, 2020

as_nested_tensor constructor doesn't behave exact the same way as as_tensor constructor. While at_tensor won't copy the data if input already is a tensor with the same metadata (device/dtype/etc) and will copy otherwise, as_nested_tensor will never copy and will throw an error in a case when it's impossible to construct a nested tensor from the given input without a copy.

Options:

1. keep everything as it is. we are agreeing that semantics are different
2. keep the behavior the same but rename as_nested_tensor to something else to avoid confusion.

pros and cons should be discussed.

@izdeby izdeby added the design label May 7, 2020
@cpuhrsch cpuhrsch changed the title as_nested_tensor doesnt behave same as as_tensor [DESIGN] as_nested_tensor doesnt behave same as as_tensor May 7, 2020
@gchanan
Copy link

gchanan commented May 7, 2020

I don't think the framing is quite right.

as_tensor turns something into a tensor if it isn't already. It's used at user-interface boundaries when you aren't sure if something is a tensor or not. The copy stuff just falls out from that.

@cpuhrsch
Copy link
Contributor

I think we need to change as_nested_tensor to what Greg suggested. If the input is already a Nestedtensor, return it. If it's not, try to create one. But having this "view" concept is troubling, because it also raises questions around things like autograd + unbind. If I unbind a NestedTensor constructed with as_nested_tensor that's tracking autograd I'm expecting to get a view that won't be a leaf, but currently I might get a leaf, since the autograd operations are simply forwarded to its constituents. This isn't an issue when the NestedTensor is contiguous as constructed by nested_tensor, since the unbound Tensors already are slices of the underlying buffer, but it is an issue when constructed with as_nested_tensor, since the unbound Tensor won't be a view (we could try to force them to be a view, but it seems smarter to make that an extension later on in context of maybe a view_as_nested_tensor operation).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants