-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add frame level task #693
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
Add frame level task #693
Conversation
8feeede
to
0c55461
Compare
34f201c
to
8e98c79
Compare
ef18a95
to
3281e66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some comments
task_index = self.get_task_index(task) | ||
if task_index not in self.tasks: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Equivalent but a bit faster and easier to read I think
task_index = self.get_task_index(task) | |
if task_index not in self.tasks: | |
if task not in self.tasks.values(): | |
task_index = self.get_task_index(task) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this logic quite confusing, so I refactor it.
dataset.add_frame({"1d": torch.randn(1)}) | ||
|
||
|
||
def test_add_frame(tmp_path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is quite long for what it's doing (~0.5s)
Did you check if this new add_frame
is slower than before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Simon Alibert <[email protected]>
Co-authored-by: Simon Alibert <[email protected]>
This reverts commit 7170819.
This reverts commit 16bb53f.
reverting commit:
Because getting:
|
9d6886d
into
user/aliberts/2025_02_10_dataset_v2.1
What this does
Motivations: We want to start supporting datasets with multiple tasks per episode. We also want to iterate on a simpler API for adding new frames to a dataset, where all features are added using
dataset.add_frame(frame: dict)
.For instance:
TODO
task
argument ofsave_episode
.add_frame
is now the only way to add the task (even if it's the same for every frame)add_frame
examples/port_datasets/pusht_zarr.py
accordinglyHow it was tested
python examples/port_datasets/pusht_zarr.py
How to checkout & try? (for the reviewer)