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

Make environment truncate correctly #8

Merged
merged 10 commits into from
Apr 25, 2024
Merged

Make environment truncate correctly #8

merged 10 commits into from
Apr 25, 2024

Conversation

rhaps0dy
Copy link
Collaborator

@rhaps0dy rhaps0dy commented Apr 15, 2024

I implemented and added tests for:

  • the environment returns truncate=True (rather than terminate=True) when the time limit is exceeded
  • when the environment gets solved at the very last step when it would be truncated, it's returned as terminated

I also fixed a previous bug:

  • Python tests failing would not make CI fail. pytest.main is not enough, one needs to explicitly check its return value and exit with it.

Copy link
Collaborator

@taufeeque9 taufeeque9 left a comment

Choose a reason for hiding this comment

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

LGTM. Left a comment regarding a test being skipped right now.

envpool/sokoban/sokoban_py_envpool_test.py Show resolved Hide resolved
envpool/sokoban/sokoban_py_envpool_test.py Outdated Show resolved Hide resolved
envpool/sokoban/sokoban_py_envpool_test.py Outdated Show resolved Hide resolved
envpool/sokoban/sokoban_py_envpool_test.py Outdated Show resolved Hide resolved
@rhaps0dy rhaps0dy enabled auto-merge April 25, 2024 05:33
@rhaps0dy rhaps0dy mentioned this pull request Apr 25, 2024
@rhaps0dy rhaps0dy disabled auto-merge April 25, 2024 06:01
@rhaps0dy rhaps0dy merged commit 497309d into sokoban Apr 25, 2024
2 checks passed
@rhaps0dy rhaps0dy deleted the truncation branch April 25, 2024 06:02
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