Skip to content

Commit 59df9b8

Browse files
Makes DataGenConfig.max_num_failures actually bound a Mimic generation run (#7433)
# Description `DataGenConfig.max_num_failures` is documented as *"Maximum number of failures allowed before stopping generation"* and eighteen shipped Mimic environment configs set it to `25`. Nothing reads it. ```console $ git grep -c max_num_failures source/isaaclab/isaaclab/envs/mimic_env_cfg.py:1 # the definition source/isaaclab_mimic/isaaclab_mimic/envs/*.py:18 # eighteen assignments ``` No third entry: there is no read anywhere in the repository, and the field has been inert since Isaac Lab Mimic was introduced in #179. `env_loop` counts `num_failures` and then never consults it; its only termination is ```python check_val = num_success if generation_guarantee else num_attempts if check_val >= generation_num_trials: ``` With `generation_guarantee = True` (which those same eighteen configs also set), a task whose success rate is low retries without any bound, and the one knob that claims to stop that does nothing. On our own data generation this cost 337 attempts for 10 demos before we discovered the cap was not real. ### Reproduction, on a stock task ```bash ./isaaclab.sh -p scripts/imitation_learning/isaaclab_mimic/generate_dataset.py \ --task Isaac-Stack-Cube-Franka-IK-Rel-Mimic-v0 \ --input_file ./datasets/annotated_dataset.hdf5 \ --output_file /tmp/out.hdf5 \ --generation_num_trials 30 --num_envs 10 --headless ``` `FrankaCubeStackIKRelMimicEnvCfg` sets `max_num_failures = 25`. Observed: | | successes | failures | attempts | |---|---|---|---| | before | 30 | **50** | 80 | | after (`max_num_failures = 25`) | 12 | **25** | 37 | Before the change the run passed the configured cap and kept going, ending at twice it. After, it stops on the attempt that reaches 25 failures and says so: ``` Reached 25 failures (max_num_failures=25) after 12/30 successes. Exiting. ``` ### On the default Wiring the field up exactly as written would abort the primary documented workflow. At the ~36% success rate measured above, `--generation_num_trials 1000` needs on the order of 1800 failures, so a cap of 25 would end the run after roughly forty attempts. Since the eighteen assignments were inert when they were written and no current behaviour depends on them, they are removed here and the default becomes `None`, meaning no limit. **Runs behave exactly as they do today unless a limit is explicitly requested.** If maintainers would rather the shipped configs keep an active cap, that is a one-line change per config and I am happy to make it — it just needs a value that suits large runs. ### Tests `source/isaaclab_mimic/test/test_generation_failure_cap.py` drives `env_loop` with a fake environment whose `step()` consumes scripted attempt outcomes and refills the action queue, so the loop's own termination logic runs unmodified with no simulator or dataset behind it; a step-count fuse turns an unbounded loop into a failed assertion. Six cases: the cap stops a run that never succeeds; `None` keeps the guarantee unbounded as before; enough successes still end the run first; a cap reached short of the target ends it; attempt-based termination with the guarantee off is untouched (×2). Against the unpatched loop the two cap-dependent cases fail (`'fuse' == 'exited'`, and `(3, 4) == (2, 4)` — the loop overran the cap and collected an extra success) and the four asserting unchanged behaviour pass. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Checklist - [x] I have read and understood the contribution guidelines - [ ] I have run the `pre-commit` checks with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JFz91VMgmrjS4XR6f9Q9Z2 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent befb2d4 commit 59df9b8

22 files changed

Lines changed: 200 additions & 20 deletions

CONTRIBUTORS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ Guidelines for modifications:
123123
* Jiwen Cai
124124
* Johnson Sun
125125
* Juana Du
126+
* Kai Pei
126127
* Kaixi Bao
127128
* Kourosh Darvish
128129
* Kousheek Chakraborty
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Fixed
2+
^^^^^
3+
4+
* Fixed :attr:`~isaaclab.envs.mimic_env_cfg.DataGenConfig.max_num_failures` being ignored. The field
5+
documented a cap on failed generation attempts but was never read, so a run with
6+
:attr:`~isaaclab.envs.mimic_env_cfg.DataGenConfig.generation_guarantee` enabled retried without
7+
bound on a task with a low success rate. Its default is now ``None`` (no limit) and setting it to
8+
an integer stops generation once that many attempts have failed.

source/isaaclab/isaaclab/envs/mimic_env_cfg.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,21 @@ class DataGenConfig:
3838
Keeping failed demonstrations is useful for visualizing and debugging low success rates.
3939
"""
4040

41-
max_num_failures: int = 50
42-
"""Maximum number of failures allowed before stopping generation."""
41+
max_num_failures: int | None = None
42+
"""Maximum number of failed generation attempts before stopping, or None for no limit.
43+
44+
Only applies together with :attr:`generation_guarantee`. With the guarantee enabled, generation
45+
keeps retrying until :attr:`generation_num_trials` demos succeed, so a task whose success rate is
46+
low can run for an unbounded number of attempts; this caps that. Defaults to None so the
47+
guarantee keeps its usual meaning unless a limit is asked for.
48+
49+
With the guarantee disabled, generation already stops after :attr:`generation_num_trials`
50+
attempts and this field is ignored, so setting it cannot cut a fixed-attempt run short.
51+
52+
The bound is read once per simulation step. Attempts that end on the step that crosses it are
53+
already complete, so a run over ``num_envs`` parallel environments can record up to
54+
``num_envs - 1`` failures beyond the bound; it is exact whenever attempts end on separate steps.
55+
"""
4356

4457
seed: int = 1
4558
"""Seed for randomization to ensure reproducibility."""
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
Fixed
2+
^^^^^
3+
4+
* Fixed the Mimic generation loop never bounding a run by failure count. ``env_loop`` now stops when
5+
``datagen_config.max_num_failures`` failed attempts have accumulated, alongside the existing stop
6+
on enough successes or attempts.
7+
8+
Removed
9+
^^^^^^^
10+
11+
* Removed the ``datagen_config.max_num_failures = 25`` assignment from the shipped Mimic environment
12+
configs. The field was never read when those lines were written, so honouring it now would newly
13+
cap every shipped task at 25 failed attempts and cut short any run asking for a large number of
14+
demos. Set the field explicitly to opt into a cap.

source/isaaclab_mimic/isaaclab_mimic/datagen/generation.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,18 @@ def env_loop(
140140
print(f"Reached {generation_num_trials} successes/attempts. Exiting.")
141141
break
142142

143+
# with the success guarantee on, nothing else bounds the run: a task that rarely
144+
# succeeds retries forever. max_num_failures is the opt-in bound on that. Without the
145+
# guarantee the check above already stops on generation_num_trials attempts, so the
146+
# cap has nothing to add and must not cut a fixed-attempt run short.
147+
max_num_failures = env.cfg.datagen_config.max_num_failures
148+
if generation_guarantee and max_num_failures is not None and num_failures >= max_num_failures:
149+
print(
150+
f"Reached {num_failures} failures (max_num_failures={max_num_failures}) after"
151+
f" {num_success}/{generation_num_trials} successes. Exiting."
152+
)
153+
break
154+
143155
# check that simulation is stopped or not
144156
if env.sim.is_stopped():
145157
break

source/isaaclab_mimic/isaaclab_mimic/envs/agibot_place_toy2box_mimic_env_cfg.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ def __post_init__(self):
3232
self.datagen_config.generation_select_src_per_subtask = True
3333
self.datagen_config.generation_transform_first_robot_pose = False
3434
self.datagen_config.generation_interpolate_from_last_target_pose = True
35-
self.datagen_config.max_num_failures = 25
3635
self.datagen_config.seed = 1
3736

3837
# The following are the subtask configurations for the stack task.

source/isaaclab_mimic/isaaclab_mimic/envs/agibot_place_upright_mug_mimic_env_cfg.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ def __post_init__(self):
2929
self.datagen_config.generation_select_src_per_subtask = True
3030
self.datagen_config.generation_transform_first_robot_pose = False
3131
self.datagen_config.generation_interpolate_from_last_target_pose = True
32-
self.datagen_config.max_num_failures = 25
3332
self.datagen_config.seed = 1
3433

3534
# The following are the subtask configurations for the stack task.

source/isaaclab_mimic/isaaclab_mimic/envs/exhaustpipe_gr1t2_mimic_env_cfg.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ def __post_init__(self):
3030
self.datagen_config.generation_joint_pos = False
3131
self.datagen_config.generation_transform_first_robot_pose = False
3232
self.datagen_config.generation_interpolate_from_last_target_pose = True
33-
self.datagen_config.max_num_failures = 25
3433
self.datagen_config.num_demo_to_render = 10
3534
self.datagen_config.num_fail_demo_to_render = 25
3635
self.datagen_config.seed = 10

source/isaaclab_mimic/isaaclab_mimic/envs/franka_bin_stack_ik_rel_mimic_env_cfg.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ def __post_init__(self):
2828
self.datagen_config.generation_transform_first_robot_pose = False
2929
self.datagen_config.generation_interpolate_from_last_target_pose = True
3030
self.datagen_config.generation_relative = True
31-
self.datagen_config.max_num_failures = 25
3231
self.datagen_config.seed = 1
3332

3433
# The following are the subtask configurations for the stack task.

source/isaaclab_mimic/isaaclab_mimic/envs/franka_stack_ik_abs_mimic_env_cfg.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ def __post_init__(self):
2727
self.datagen_config.generation_select_src_per_subtask = True
2828
self.datagen_config.generation_transform_first_robot_pose = False
2929
self.datagen_config.generation_interpolate_from_last_target_pose = True
30-
self.datagen_config.max_num_failures = 25
3130
self.datagen_config.seed = 1
3231

3332
# The following are the subtask configurations for the stack task.

0 commit comments

Comments
 (0)