Some fixes to NLE and the competition task #173
Conversation
facebook-github-bot
added
the
CLA Signed
The {a,b,c} syntax appears to not be supported:
https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet
heiner
force-pushed
the
heiner/fixcompenv
branch
from
5f5e10f to
2bd281c
Compare
There was a problem hiding this comment.
The code all looks fine except I wouldnt mark this for release yet. Some discussion points listed. The only think I'm thinking about is: do we whole hog overhaul action space or make an incremental change? Another option (crazy) is to provide mapping to old action space in some way?
| @@ -341,6 +341,13 @@ def __init__( | |||
| # If the in-game turn count doesn't change for 10_000 steps, we abort | |||
| self.no_progress_timeout = no_progress_timeout | |||
|
|
|||
| def f(*args, **kwargs): | |||
There was a problem hiding this comment.
Given the discussions previously: do we need this monkey-patching? And should it happen here? I think given the AICrowd set up we probably dont NEED to prevent this here, and the trade off is: "being precautionary" vs preventing people from using seeds some how in training (Prioritised Level Replay?).
However, I'm pretty precautionary kinda guy, and would opt for keeping this. I guess the main thing I'm interested in is: how is this meant to work with the other compilation changes? If seeding is switched off, are these exceptions just signposting? If so: why do we only do that here, and not in all the environments?
There was a problem hiding this comment.
Ah i have seen below now that the compilation changes result in thrown exceptions in pynethack.cc so this really is window dressing, presumably to be extra clear to competitors?
There was a problem hiding this comment.
History of this change: I wanted to remove the Python functions that do seed setting, but unfortunately the pybind11 object's attributes are read-only so they cannot be changed this way.
I guess we could not do this now.
heiner
force-pushed
the
heiner/fixcompenv
branch
from
1cd9ab9 to
67182a4
Compare
This commit is rather defensive for purposes of the NLE competition. Afterwards, we can #define NLE_ALLOW_SEEDING 1 to turn on the old behaviour.
heiner
force-pushed
the
heiner/fixcompenv
branch
from
67182a4 to
e830174
Compare
|
Updated PR to not touch actions for now. |
This PR fixes our C++ lint test (which apparently never really worked and at any rate was broken for months now) and flat-out disables seeding in NLE. The seeding commit is rather defensive for purposes of the NLE competition. Afterwards, we can
#define NLE_ALLOW_SEEDING 1to turn on the old behaviour.