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

Does changing the observation space seems to require changing C code? #211

Closed
dmadeka opened this issue Jul 13, 2021 · 2 comments
Closed
Assignees

Comments

@dmadeka
Copy link
Contributor

dmadeka commented Jul 13, 2021

Right now, the observation_keys argument is compared against OBSERVATION_DESC (https://github.com/facebookresearch/nle/blob/master/nle/nethack/nethack.py#L145) which is pinned against the set_buffers in (https://github.com/facebookresearch/nle/blob/master/win/rl/pynethack.cc#L126)

The gym observation space is also defined from observation_keys. So whats the intended design for adding something to the observation space like an occupancy map?

The current solution looks like (is this the intended design?):

class NetHackOccupancyMap(NetHackScore):
    def __init__(self, *args, **kwargs):
            super().__init__(*args, **kwargs)
            self.positions_visited = {(0, 1):
                        np.zeros((21, 79))
                    }

            occupancy_map_tuple = ("occupancy_map",
                                gym.spaces.Box(low=0, high=nethack.MAX_GLYPH, **nethack.OBSERVATION_DESC["glyphs"]))

            space_dict = kwargs.pop("space_dict", None)
            if not space_dict:
                space_dict = dict(NLE_SPACE_ITEMS + (occupancy_map_tuple,))
            else:
                space_dict = dict(space_dict + (occupancy_map_tuple,))
    
            observation_keys_new = kwargs.pop("observation_keys", (
                "glyphs",
                "chars",
                "colors",
                "specials",
                "blstats",
                "message",
                "inv_glyphs",
                "inv_strs",
                "inv_letters",
                "inv_oclasses",
                "screen_descriptions",
                "tty_chars",
                "tty_colors",
                "tty_cursor",
                "occupancy_map",
            ))
        
            self.observation_space = gym.spaces.Dict(
                {key: space_dict[key] for key in observation_keys_new}
            )
          
@cdmatters cdmatters self-assigned this Jul 15, 2021
@cdmatters
Copy link
Contributor

Hi @dmadeka ,

Thanks for opening the issue. You seem to want to extend NLE with a new observation and a new task. I will ignore the latter as there are couple of examples you can check in nle/env/tasks.py, and just address the observation.

If you wish to add a new observation you need to first ask what are you trying to observe. Is this a new piece of state that carries new internal information out of NetHack? Or is this a new observation derived from existing state that has already been exposed by the NLE.

If the former, I suggest looking at the recent commit that added the misc observation to NLE. Here is the commit and PR.

If the latter, then I suggest writing a wrapper (or simply a new task) that wraps around that intercepts the step and reset functions to add more observations. I'm sure there are examples of this elsewhere online since you would just need to follow the gym API.

If this answers doesn't answer your question feel free to reopen the issue.


( Finally on a personal note @dmadeka - I want to thank you for you contributions, you are very close to merging your contribution to NLE (PR #202 )! All you need to do is pass the linting tests! )

@dmadeka
Copy link
Contributor Author

dmadeka commented Jul 19, 2021

Reopened #217 correctly and fixed the linting issues

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

No branches or pull requests

2 participants