Skip to content
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.

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

[Feature Request] tensordict.entry_dtype(entry), tensordict.entry_device(entry) #169

Open
vmoens opened this issue Jan 18, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@vmoens
Copy link
Contributor

vmoens commented Jan 18, 2023

Motivation

In #168 we introduce tensordict._entry_class. Maybe it would make sense to have the same for dtype and device to avoid doing things like if tensordict.get(key).dtype is torch.float32.
These methods should be made public and documented.
We should find a proper way to do it with lazy tensordicts (especially stack). One option would be to take the first tensordict of the list and query that method on it, implicitely assuming that homogeneous tensordicts have been stacked. This is prone bug if one stacks tensordicts with varying dtypes but that's a behaviour that is not supported anyway (as usual: do we want to apply expensive checks or just tell the users to be cautious because we don't?)

cc @tcbegley

cc @matteobettini fyi since this implies lazy-stacked tds

@vmoens vmoens added the enhancement New feature or request label Jan 18, 2023
@vmoens vmoens self-assigned this Jan 18, 2023
@matteobettini
Copy link
Contributor

matteobettini commented Jan 18, 2023

Right now is it allowed to stack tensordicts with different shapes and different dtypes or devices?

If so, I think we should throw an error for any call to entry_dtype or entry_device on heterogenous shape stacks, behaving in the same way that tensordict.get(entry) does. (It will not matter if they actually have same device or dtype)

If not, that's already enough. We know that they have the same dtype ot device and we can jsut look at the first or smth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants