Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

Forward wizard mode through nle, add tests. #64

Merged
merged 2 commits into from
Sep 21, 2020
Merged

Forward wizard mode through nle, add tests. #64

merged 2 commits into from
Sep 21, 2020

Conversation

yobibyte
Copy link
Contributor

No description provided.

@yobibyte yobibyte added the gym label Sep 18, 2020
@yobibyte yobibyte requested a review from heiner September 18, 2020 15:54
@yobibyte yobibyte self-assigned this Sep 18, 2020
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 18, 2020
Copy link
Contributor

@heiner heiner left a comment

Choose a reason for hiding this comment

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

Thanks!

Small question about the test below.

@@ -75,25 +75,26 @@ def compare_rollouts(env0, env1, max_rollout_len):


@pytest.mark.parametrize("env_name", get_nethack_env_ids())
@pytest.mark.parametrize("wizard", [False, True])
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the elaborate test!

I'm a little bit reluctant though to double the runtime of the test like this, although obviously it gives optimal test coverage this way. Perhaps we could not add wizard to TestGymEnvRollout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was not sure what wizard-mode affects under the hood, that's why I added the option. I can remove not to increase the testing time.

@rockt rockt self-requested a review September 18, 2020 17:34
Copy link
Contributor

@heiner heiner left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good.

@yobibyte yobibyte merged commit 2f350c9 into master Sep 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. gym
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants