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.

Sign up for GitHub

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

Jump to bottom

[Feature] Cat LazyStackedTensorDicts #499

Merged
merged 16 commits into from
Aug 2, 2023
Merged

[Feature] Cat LazyStackedTensorDicts #499

merged 16 commits into from
Aug 2, 2023

Conversation

matteobettini
Copy link
Contributor

No description provided.

@matteobettini
amend
fb305fe 
Signed-off-by: Matteo Bettini <[email protected]>
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 26, 2023
@matteobettini
amend
bedd19b 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini
amend
2f73032 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini matteobettini marked this pull request as ready for review July 26, 2023 16:23
@matteobettini matteobettini mentioned this pull request Jul 26, 2023
Merged
@vmoens vmoens changed the title [Feature] Cat lazy tds [Feature] Cat LazyStackedTensorDicts Jul 27, 2023
vmoens
vmoens approved these changes Jul 27, 2023
View reviewed changes
Copy link
Contributor

@vmoens vmoens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM some minor comments

tensordict/tensordict.py Outdated Show resolved Hide resolved
tensordict/tensordict.py Outdated Show resolved Hide resolved
tensordict/tensordict.py Outdated Show resolved Hide resolved
tensordict/tensordict.py Outdated Show resolved Hide resolved
tensordict/tensordict.py Outdated Show resolved Hide resolved
tensordict/tensordict.py Outdated Show resolved Hide resolved
matteobettini and others added 4 commits July 27, 2023 16:45
@matteobettini
amend
f39bd8e 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini @vmoens
Apply suggestions from code review
393ae57 
Co-authored-by: Vincent Moens <[email protected]>
@matteobettini
remove key check
0d93020 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini
update inplace
7f5c564 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini
Copy link
Contributor Author

matteobettini commented Jul 27, 2023 •
edited
Loading

i changed the out=smth branch to use update(, inplace=True)

@matteobettini
Merge branch 'main' into lazy_cat
4327c97
@matteobettini
amend
a76c238 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini
amend
eb07c43 
Signed-off-by: Matteo Bettini <[email protected]>
vmoens
vmoens reviewed Jul 28, 2023
View reviewed changes
test/test_tensordict.py Show resolved Hide resolved
tensordict/tensordict.py Outdated Show resolved Hide resolved
@matteobettini
amend
d7fce9d 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini
amend
a25401e 
Signed-off-by: Matteo Bettini <[email protected]>
@vmoens vmoens added the enhancement New feature or request label Jul 28, 2023
@matteobettini
Merge branch 'main' into lazy_cat
dffa92e
@matteobettini
amend
36df8a8 
Signed-off-by: Matteo Bettini <[email protected]>
@matteobettini
amend
82ebac5 
Signed-off-by: Matteo Bettini <[email protected]>
vmoens
vmoens approved these changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@vmoens vmoens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment then good to go

tensordict/tensordict.py Outdated Show resolved Hide resolved
@matteobettini
amend
6c05c39 
Signed-off-by: Matteo Bettini <[email protected]>
@vmoens vmoens merged commit a7be2f4 into pytorch:main Aug 2, 2023
@matteobettini matteobettini deleted the lazy_cat branch August 2, 2023 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vmoens vmoens vmoens approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@matteobettini @vmoens @facebook-github-bot