Skip to content

Commit b584311

Browse files
authored
fix(vla): count only terminated as success and reject multi-env in OpenVLA eval (#787)
The SmolVLA and pi0 eval loops set stats['success']=True on terminated OR truncated, so a timeout (truncated) was recorded as a success, inflating the reported success rate. Count only terminated as success; truncated just ends the episode (matches the OpenVLA eval convention). OpenVLA's predict_action only consumes env 0's image and emits a single action, so num_envs>1 silently produced a wrong-shaped action. Raise in __init__ (before the model load) when num_envs != 1, matching pi_eval. Adds general source-guard regression tests (the eval loops need a model+env, so not functionally runnable here). Red on pre-fix. Note: these eval files carry heavy pre-existing ruff-format debt that is intentionally left untouched.
1 parent 2d37024 commit b584311

4 files changed

Lines changed: 56 additions & 2 deletions

File tree

roboverse_learn/vla/OpenVLA/vla_eval.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ def __init__(
5757
self.env = env
5858
self.scenario = scenario
5959
self.num_envs = num_envs
60+
# predict_action only consumes env 0's image and emits a single action,
61+
# so num_envs > 1 silently produces a wrong-shaped action. Fail fast
62+
# (matches pi_eval). Placed before _init_policy so it raises without a
63+
# model load.
64+
if num_envs != 1:
65+
raise ValueError("vla_eval currently supports num_envs == 1")
6066
self.device = device
6167
self.task_name = task_name
6268
self.robot_name = robot_name

roboverse_learn/vla/SmolVLA/smolvla_eval.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,12 @@ def evaluate_episode(
365365
is_terminated = terminated.any().item() if isinstance(terminated, torch.Tensor) else terminated
366366
is_truncated = truncated.any().item() if isinstance(truncated, torch.Tensor) else truncated
367367

368-
if is_terminated or is_truncated:
368+
# Only terminated=True is success; truncated=True is a timeout (failure).
369+
if is_terminated:
369370
stats["success"] = True
370371
break
372+
elif is_truncated:
373+
break
371374

372375
obs_saver.save()
373376

roboverse_learn/vla/pi0/pi_eval.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,12 @@ def evaluate_episode(
251251

252252
term = terminated.any().item() if hasattr(terminated, "any") else bool(terminated)
253253
trunc = truncated.any().item() if hasattr(truncated, "any") else bool(truncated)
254-
if term or trunc:
254+
# Only terminated=True is success; truncated=True is a timeout (failure).
255+
if term:
255256
stats["success"] = True
256257
break
258+
elif trunc:
259+
break
257260

258261
obs_saver.save()
259262

tests/test_vla_eval_fixes.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
"""Regression: VLA eval success metric and OpenVLA num_envs guard.
2+
3+
- SmolVLA and pi0 eval loops set ``stats["success"] = True`` on ``terminated OR
4+
truncated`` — a timeout (truncated) was counted as a success, inflating the
5+
reported success rate. Only ``terminated`` is success (OpenVLA gets this right).
6+
- OpenVLA's predict_action only consumes env 0's image and emits one action, so
7+
``num_envs > 1`` silently produces a wrong-shaped action; the runner must fail
8+
fast.
9+
10+
The eval loops need a model + env to run, so these are source-level guards.
11+
"""
12+
13+
from __future__ import annotations
14+
15+
import pathlib
16+
17+
import pytest
18+
19+
_VLA = pathlib.Path(__file__).resolve().parents[1] / "roboverse_learn" / "vla"
20+
21+
22+
@pytest.mark.general
23+
def test_smolvla_eval_only_terminated_is_success():
24+
src = (_VLA / "SmolVLA" / "smolvla_eval.py").read_text()
25+
assert "if is_terminated or is_truncated:" not in src, "timeout must not be counted as success"
26+
assert "if is_terminated:" in src
27+
assert "elif is_truncated:" in src
28+
29+
30+
@pytest.mark.general
31+
def test_pi0_eval_only_terminated_is_success():
32+
src = (_VLA / "pi0" / "pi_eval.py").read_text()
33+
assert "if term or trunc:" not in src, "timeout must not be counted as success"
34+
assert "if term:" in src
35+
assert "elif trunc:" in src
36+
37+
38+
@pytest.mark.general
39+
def test_openvla_eval_rejects_multi_env():
40+
src = (_VLA / "OpenVLA" / "vla_eval.py").read_text()
41+
assert "if num_envs != 1:" in src
42+
assert "raise ValueError" in src

0 commit comments

Comments
 (0)