Skip to content

Commit 548ed49

Browse files
authored
fix(il): restore random val split and shape assert in sampler (#776)
get_val_mask hardcoded `val_idxs = -1` (the real `rng.choice(...)` selection was commented out), so the validation set was always exactly the last episode regardless of val_ratio/seed — every reported IL validation metric was computed on one fixed episode. Restore the seeded random selection of n_val episodes. create_indices had `episode_mask.shape == episode_ends.shape` as a bare no-op expression (missing the `assert` keyword), so the shape precondition never ran. Restore the assert (numba-nopython compatible; the sole caller always builds matching shapes, so it never trips in practice but fails fast on misuse). Adds general regression tests: val mask selects round(n*ratio) episodes and is seed-reproducible; create_indices asserts on a shape mismatch. Red on pre-fix.
1 parent 582c348 commit 548ed49

2 files changed

Lines changed: 38 additions & 3 deletions

File tree

roboverse_learn/il/utils/sampler.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def create_indices(
2525
pad_after: int = 0,
2626
debug: bool = True,
2727
) -> np.ndarray:
28-
episode_mask.shape == episode_ends.shape
28+
assert episode_mask.shape == episode_ends.shape
2929
pad_before = min(max(pad_before, 0), sequence_length - 1)
3030
pad_after = min(max(pad_after, 0), sequence_length - 1)
3131

@@ -68,8 +68,7 @@ def get_val_mask(n_episodes, val_ratio, seed=0):
6868
# have at least 1 episode for validation, and at least 1 episode for train
6969
n_val = min(max(1, round(n_episodes * val_ratio)), n_episodes - 1)
7070
rng = np.random.default_rng(seed=seed)
71-
# val_idxs = rng.choice(n_episodes, size=n_val, replace=False)
72-
val_idxs = -1
71+
val_idxs = rng.choice(n_episodes, size=n_val, replace=False)
7372
val_mask[val_idxs] = True
7473
return val_mask
7574

tests/test_il_numba_optional.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,39 @@ def test_create_indices_runs_without_numba():
6666
assert len(indices) > 0
6767
# buffer windows never exceed the data length.
6868
assert indices[:, 1].max() <= episode_ends[-1]
69+
70+
71+
@pytest.mark.general
72+
def test_get_val_mask_selects_n_val_episodes():
73+
"""Regression: the validation split must select ~val_ratio episodes, not just one.
74+
75+
The real selection was commented out and replaced with ``val_idxs = -1`` so
76+
the val set was always exactly the last episode regardless of val_ratio/seed.
77+
"""
78+
from roboverse_learn.il.utils.sampler import get_val_mask
79+
80+
n_episodes, val_ratio = 10, 0.2
81+
mask = get_val_mask(n_episodes, val_ratio, seed=0)
82+
assert mask.sum() == 2, f"expected round(10*0.2)=2 val episodes, got {int(mask.sum())}"
83+
# Not degenerate-to-last: with seed=0 the chosen set must not be only the last index.
84+
assert not (mask[-1] and mask.sum() == 1)
85+
86+
# Reproducible under a fixed seed.
87+
assert np.array_equal(mask, get_val_mask(n_episodes, val_ratio, seed=0))
88+
# val_ratio <= 0 -> empty val set (unchanged contract).
89+
assert get_val_mask(n_episodes, 0.0).sum() == 0
90+
91+
92+
@pytest.mark.general
93+
def test_create_indices_asserts_shape_mismatch():
94+
"""Regression: the shape precondition must actually assert (it was a no-op expr).
95+
96+
With episode_mask longer than episode_ends, the pre-fix code silently ran to
97+
completion; the restored ``assert`` makes it fail fast.
98+
"""
99+
from roboverse_learn.il.utils.sampler import create_indices
100+
101+
episode_ends = np.array([5, 10], dtype=np.int64)
102+
episode_mask = np.ones(3, dtype=bool) # deliberately mismatched (len 3 vs 2)
103+
with pytest.raises(AssertionError):
104+
create_indices(episode_ends, sequence_length=3, episode_mask=episode_mask, debug=False)

0 commit comments

Comments
 (0)