Summary
TaskManagerConfig.max_bad_states is documented as restarting the simulator after a run of consecutive bad episodes (for example, the agent leaving the expected app). The TaskManager side of that path runs: it counts bad episodes, logs that a restart is happening, and increments restart stats. The actual simulator relaunch never happens, because the flag it sets is never read by the Coordinator (or anything else).
Current behavior
When DumpsysThread reports that the user exited the expected app, TaskManager._determine_transition_fn() calls _increment_bad_state() and returns a truncation:
https://github.com/google-deepmind/android_env/blob/main/android_env/components/task_manager.py
In _increment_bad_state():
_bad_state_counter is incremented.- Once
counter >= max_bad_states (default 3), it logs:
Too many consecutive bad states. Restarting simulator.
stats['restart_count_max_bad_states'] is incremented.self._should_restart = True is set.
The consecutive-counter logic itself looks intentional and consistent with the config:
reset_task() only clears _bad_state_counter when the previous episode was not a bad episode, so consecutive bad episodes accumulate as documented.TaskManagerConfig.max_bad_states is documented as: if that many episodes finish in a bad state in a row, restart the simulation.
The gap is on the Coordinator side. Coordinator.rl_reset() only relaunches when the simulator is unhealthy or a periodic restart is due:
if not self._simulator_healthy or self._should_periodic_relaunch():
self._launch_simulator()
It never consults the TaskManager bad-state signal. A search of the repo shows _should_restart is written in exactly one place and has no readers. It is also never initialized in TaskManager.__init__, and there are no unit tests for this restart path in task_manager_test.py or coordinator_test.py.
Net effect after N consecutive "user exited" truncations:
- Logs claim the simulator is restarting.
restart_count_max_bad_states increases.- The emulator process is left running; only a normal episode reset runs.
Expected behavior
Per TaskManagerConfig and the docstring on _increment_bad_state, after max_bad_states consecutive bad episodes the simulator should be relaunched, in the same recovery class as periodic restart / unhealthy-simulator relaunch. Stats and logs should reflect a restart that actually occurred.
Impact
For long-running training, repeated activity-check / "user exited" failures currently only truncate episodes. The recovery path that max_bad_states is meant to provide never fires, so the environment can stay in a stale or sticky bad UI state. The existing log line and restart_count_max_bad_states counter also make debugging misleading, because they report restarts that did not happen.
Suggested fix
I think this is unfinished wiring rather than a design change. A small fix that mirrors the existing periodic-restart path:
- Initialize the flag in
TaskManager and expose it (for example a should_restart() getter).
- In
Coordinator.rl_reset(), also call _launch_simulator() when that flag is set, alongside the existing unhealthy / periodic checks.
- Clear the flag after a successful relaunch (and reset the bad-state counter as appropriate).
- Add unit tests for the TaskManager signal and for Coordinator picking it up on reset.
Happy to open a PR along those lines if this matches the intended design.
Summary
TaskManagerConfig.max_bad_statesis documented as restarting the simulator after a run of consecutive bad episodes (for example, the agent leaving the expected app). The TaskManager side of that path runs: it counts bad episodes, logs that a restart is happening, and increments restart stats. The actual simulator relaunch never happens, because the flag it sets is never read by the Coordinator (or anything else).Current behavior
When
DumpsysThreadreports that the user exited the expected app,TaskManager._determine_transition_fn()calls_increment_bad_state()and returns a truncation:https://github.com/google-deepmind/android_env/blob/main/android_env/components/task_manager.py
In
_increment_bad_state():_bad_state_counteris incremented.counter >= max_bad_states(default 3), it logs:Too many consecutive bad states. Restarting simulator.stats['restart_count_max_bad_states']is incremented.self._should_restart = Trueis set.The consecutive-counter logic itself looks intentional and consistent with the config:
reset_task()only clears_bad_state_counterwhen the previous episode was not a bad episode, so consecutive bad episodes accumulate as documented.TaskManagerConfig.max_bad_statesis documented as: if that many episodes finish in a bad state in a row, restart the simulation.The gap is on the Coordinator side.
Coordinator.rl_reset()only relaunches when the simulator is unhealthy or a periodic restart is due:It never consults the TaskManager bad-state signal. A search of the repo shows
_should_restartis written in exactly one place and has no readers. It is also never initialized inTaskManager.__init__, and there are no unit tests for this restart path intask_manager_test.pyorcoordinator_test.py.Net effect after N consecutive "user exited" truncations:
restart_count_max_bad_statesincreases.Expected behavior
Per
TaskManagerConfigand the docstring on_increment_bad_state, aftermax_bad_statesconsecutive bad episodes the simulator should be relaunched, in the same recovery class as periodic restart / unhealthy-simulator relaunch. Stats and logs should reflect a restart that actually occurred.Impact
For long-running training, repeated activity-check / "user exited" failures currently only truncate episodes. The recovery path that
max_bad_statesis meant to provide never fires, so the environment can stay in a stale or sticky bad UI state. The existing log line andrestart_count_max_bad_statescounter also make debugging misleading, because they report restarts that did not happen.Suggested fix
I think this is unfinished wiring rather than a design change. A small fix that mirrors the existing periodic-restart path:
TaskManagerand expose it (for example ashould_restart()getter).Coordinator.rl_reset(), also call_launch_simulator()when that flag is set, alongside the existing unhealthy / periodic checks.Happy to open a PR along those lines if this matches the intended design.