Skip to content

Conversation

@pckSF
Copy link
Contributor

@pckSF pckSF commented Sep 10, 2025

Currently, when generating episode data with EpisodeGenerator, an assertion error

terminals and timeouts never become True at the same time

is raised if an episode terminates in the same timestep at which truncation occurs.

However, it is possible for an environment to terminate in the same timestep in which the TimeLimit wrapper truncates an episode. In this case, the environment’s step method (see, e.g., Humanoid-v5) has already returned a termination signal, which the TimeLimit wrapper simply passes through while adding its truncation flag. As a result, both termination and truncation can be True in the same timestep.

To handle this overlap, I suggest giving precedence to termination. The rationale is:

  • Termination is the primary end condition: if the environment signals termination, truncation is redundant.
  • Treating the final step as a termination (rather than truncation) makes that last transition usable for learning, adding one more usable timestep.

Concretely, the assertion

assert (
    np.sum(np.logical_and(terminals, timeouts)) == 0
), "terminals and timeouts never become True at the same time"

is replaced with:

if np.sum(np.logical_and(terminals, timeouts)) != 0:
   timeouts[terminals.astype(np.bool_)] = 0.0

While the practical effect of this change is limited — since the TransitionPicker discards the final timestep of each episode — it prevents assertion failures when constructing datasets from trajectories that contain termination–truncation overlaps.


Another option would be to remove

assert (
    np.sum(np.logical_and(terminals, timeouts)) == 0
), "terminals and timeouts never become True at the same time"

This way, timeout information stays in the _timeouts attribute of the EpisodeGenerator, keeping it consistent with the environment’s return values, but deviating from the implied intention of the assertion if it was meant to be more than just a consistency check.

Copy link
Owner

@takuseno takuseno left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for your contribution!

@takuseno takuseno merged commit 38c34b6 into takuseno:master Sep 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants