Skip to content

Add absolute_to_reative_idx for remapping indices to fix delta_timestamp bug#2490

Merged
michel-aractingi merged 1 commit into
mainfrom
fix/relative_indices_bug
Nov 20, 2025
Merged

Add absolute_to_reative_idx for remapping indices to fix delta_timestamp bug#2490
michel-aractingi merged 1 commit into
mainfrom
fix/relative_indices_bug

Conversation

@michel-aractingi
Copy link
Copy Markdown
Contributor

@michel-aractingi michel-aractingi commented Nov 20, 2025

What this does

Bug reported by community in https://discord.com/channels/1216765309076115607/1438871716792373248/1441012729342197911

When delta_timestamps are used with filtered episodes (subset of the dataset is only loaded), the code was trying to access the dataset using absolute indices from the full dataset, but the filtered dataset only contains a subset of rows.

FIX: Add an index mapping that from absolute indices to local dataset indices ONLY when a subset of the dataset is requested.

Testing

Tested with the code snipper provided by the user. Before this PR this test fails:

from lerobot.datasets.lerobot_dataset import LeRobotDataset
import torch
repo_id = "aractingi/droid_1.0.1"
episodes = [2, 3, 4, 5]
delta_timestamps = {
    "observation.images.exterior_1_left": [-1, 10/15, 5/15, 0]
}

dataset = LeRobotDataset(repo_id, episodes=episodes, delta_timestamps=delta_timestamps)
print(dataset)
dataloader = torch.utils.data.DataLoader(dataset, batch_size=16, shuffle=True)
for batch in dataloader:
    print(f"Received batch index {batch['index']}", end="\r")

Copilot AI review requested due to automatic review settings November 20, 2025 12:22
Copilot AI reviewed Nov 20, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where delta_timestamps failed when used with filtered episodes (loading only a subset of episodes). The issue occurred because query indices computed from episode metadata used absolute dataset indices, but these were being used to access a filtered dataset that only contained relative indices.

  • Adds an _absolute_to_relative_idx mapping dictionary that translates absolute dataset indices to relative positions in the filtered dataset
  • Applies the index mapping in _get_query_timestamps and _query_hf_dataset methods when querying data
  • Ensures the mapping is only created when episodes are filtered (self.episodes is not None)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jadechoghari
jadechoghari approved these changes Nov 20, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@jadechoghari jadechoghari left a comment

Choose a reason for hiding this comment

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

lgtm

@michel-aractingi michel-aractingi merged commit 0f551df into main Nov 20, 2025
16 checks passed
@michel-aractingi michel-aractingi deleted the fix/relative_indices_bug branch November 20, 2025 13:05
@michel-aractingi michel-aractingi mentioned this pull request Nov 26, 2025
Closed
pzal pushed a commit to pzal/lerobot that referenced this pull request Dec 2, 2025
@michel-aractingi @pzal
add absolute_to_reative_idx for remapping indicies when a subset of…
9631e8f 
… data is loaded (huggingface#2490)
@atyshka
Copy link
Copy Markdown
Contributor

atyshka commented Dec 9, 2025

@michel-aractingi @jadechoghari I don't think this fix is working. Here is a minimal test case:

from lerobot.datasets.lerobot_dataset import LeRobotDataset

delta_ts = {
    "observation.image":[0.0]
}
dataset = LeRobotDataset("lerobot/pusht", episodes=[1], delta_timestamps=delta_ts)

for frame in dataset:
    print(frame["observation.image_is_pad"])

Result: Every frame from episode 1 is marked as a pad. You can also confirm that the frame itself is invalid.

If you instead set episodes to [0] or [0,1], the frames are correctly sampled.

@atyshka atyshka mentioned this pull request Dec 9, 2025
Merged
XHAKA3456 pushed a commit to XHAKA3456/lerobot that referenced this pull request Dec 12, 2025
@michel-aractingi @XHAKA3456
add absolute_to_reative_idx for remapping indicies when a subset of…
0fc148c 
… data is loaded (huggingface#2490)
sandhya-cb pushed a commit to sandhya-cb/lerobot-clutterbot that referenced this pull request Jan 28, 2026
@michel-aractingi @sandhya-cb
add absolute_to_reative_idx for remapping indicies when a subset of…
dd0f8ab 
… data is loaded (huggingface#2490)
lu391see pushed a commit to lu391see/lerobot_tactile that referenced this pull request Mar 24, 2026
@michel-aractingi @lu391see
add absolute_to_reative_idx for remapping indicies when a subset of…
d119edb 
… data is loaded (huggingface#2490)
@hello3x3 hello3x3 mentioned this pull request Apr 3, 2026
Merged
4 tasks
@hello3x3
Copy link
Copy Markdown
Contributor

hello3x3 commented Apr 3, 2026

I found that _absolute_to_reative_idx materializes the entire column as Python / tensor objects becomes a significant bottleneck for large datasets. Use .to_numpy() is much faster. Have you noticed this?

Please review #3279 , thank you for your great job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@jadechoghari jadechoghari jadechoghari approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@michel-aractingi @atyshka @hello3x3 @jadechoghari