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

Implemented Operations between two TensorDicts are defined by their position in their respective trees, not their keys #853

Closed
ludwigwinkler opened this issue Jul 5, 2024 · 1 comment · Fixed by #855
Assignees
Labels
bug Something isn't working

Comments

@ludwigwinkler
Copy link

ludwigwinkler commented Jul 5, 2024

Describe the bug

For example, the add function is implemented between two tensordicts.
Yet which of the leaves of two tensordicts are added is determined by their position in the respective tensordicts, not their explicit keys.

To Reproduce

import torch
from tensordict import TensorDict

x0 = TensorDict(
    {
        "y": torch.tensor([1.0, 0.1, -0.1]),
        "x": torch.tensor([-1.0, 0.1, 0.0]),
    }
)

x1 = TensorDict(
    {
        "x": torch.tensor([-1.0, 0.1, 0.0]),
        "y": torch.tensor([2.0, 0.1, -0.1]),
    }
)

print(f"+: ", (x0 + x1).to_dict())
print(f"*: ", (x0 * x1).to_dict())

Output:

+:  {'y': tensor([ 0.0000,  0.2000, -0.1000]), 'x': tensor([ 1.0000,  0.2000, -0.1000])}
*:  {'y': tensor([-1.0000,  0.0100, -0.0000]), 'x': tensor([-2.0000,  0.0100, -0.0000])}

Here the leaves are processed irrespective of their key.
Thus position in the tree trumps the keys.

Additional context

I just became aware of tensordict.apply() which should probably be used instead and implements a tree_map() functionality?

As far as I can tell, maybe the add functionality here should use an items() iterator which sortes the values by their keys?

def add(self, other: TensorDictBase | float, alpha: float | None = None):

@ludwigwinkler ludwigwinkler added the bug Something isn't working label Jul 5, 2024
@vmoens
Copy link
Contributor

vmoens commented Jul 5, 2024

On it

@ludwigwinkler ludwigwinkler changed the title Implemented Operations between two TensorDicts is defined by their position in their respective trees, not their keys Implemented Operations between two TensorDicts are defined by their position in their respective trees, not their keys Jul 5, 2024
@vmoens vmoens linked a pull request Jul 5, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants