Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal/Bug Fix] Change truncation to termination in Car Racing after finishing a lap #106

Open
RedTachyon opened this issue Nov 2, 2022 · 2 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@RedTachyon
Copy link
Member

RedTachyon commented Nov 2, 2022

Proposal

Currently in Car Racing, when the agent finishes a lap, the environment is marked as truncated instead of terminated. This seems like a really odd choice to me.

if self.tile_visited_count == len(self.track) or self.new_lap:
# Truncation due to finishing lap
# This should not be treated as a failure
# but like a timeout
truncated = True

This was added in openai/gym#2890 alongside an actual fix to the environment logic. I suspect the review focused on the bug fix, and omitted the undiscussed change, so it slipped through the cracks. (BTW now you see why I'm always being annoying about out-of-scope changes in PRs and similar stuff)

Finishing a lap is a very clear example for episode termination. You reach a terminal state after making a full loop, and the episode ends. It should never have been marked as truncation.

The annoying part the environment version was bumped for this (but also for the actual bug), so we'll have to bump it again. But I can't really see any justification for keeping this marked as truncation, which is inconsistent with the entire rationale for what truncation is meant to be (reaching the time limit). The explanation in some of the comments was that finishing the lap shouldn't be treated as a failure, but termination does not imply failure. Failure or success is defined by the reward. Termination says "You're done, nothing more to do". Truncation says "You took too long, try again".

@pseudo-rnd-thoughts @jkterry1 @araffin

@RedTachyon RedTachyon added enhancement New feature or request bug Something isn't working labels Nov 2, 2022
@Markus28
Copy link
Contributor

Markus28 commented Nov 3, 2022

I don't think it's an MDP if you change it to be a termination since the observation doesn't really carry any information about whether a lap has been completed/how much of a lap has been completed (or does it, idk?). So making it a termination may be problematic, at least that would be my understanding.

@RedTachyon
Copy link
Member Author

I don't think it's an MDP if you change it to be a termination

I don't really buy this. The observation is a pixel rendering in a radius around the environment, so it's hardly an MDP in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants