Save ttyrecs every "M" episode

[dmadeka]

Currently the base environment seems to write a ttyrec at every call of reset (if savedir is not None). This creates a lot of ttyrec files.

It may be useful to add a parameter (say save_episode_cycle) to determine when to save a ttyrec - the ttyrec in that line can be set to None for every other episode in the cycle

Would this be a PR you alls would be interested in?

[heiner]

Hey!

This is a good idea but should probably not be done on the python side. The reason is that even when writing every 1000th episode only, we'd want to keep the existing ttyrec output file open. (This issue does not apply if you are looking to write different ttyrec files anyway.)

[dmadeka]

@deanpfoster and I were thinking of something like this here:

        if self._episode % self._episode_save_cycle == 0:
            self.last_observation = self.env.reset(new_ttyrec, wizkit_items=wizkit_items)
        else:
            self.last_observation = self.env.reset('/dev/null', wizkit_items=wizkit_items)

[heiner]

So if one is changing the new_ttyrec variable anyway, that's fine. It seems we are doing that in base.py, so no issue with that.

I would suggest not using the "/dev/null" path. We used to do that, but that means bz2 compresses the ttyrec output only for it to be then discarded. That was actually 95%+ of NLE's compute wasted right there :D

Instead, we have a code path for None as the ttyrec setting, which avoids parsing things altogether: https://github.com/facebookresearch/nle/blob/main/nle/nethack/nethack.py#L254

[dmadeka]

I first thought of replacing “/dev/null” with ‘None’ - but then realized it might be the open file stream issue you were mentioning?

[heiner]

I might have been a bit unclear regarding what the "file stream issue" is.

If you look at https://github.com/facebookresearch/nle/blob/main/win/rl/pynethack.cc#L108 and https://github.com/facebookresearch/nle/blob/main/src/nle.c#L125 you see that the C++ layer in pynethack.cc opens FILEs with fopen/fclose and then calls nle_reset with some arguments. The C layer in nle.c checks if its ttyrec FILE is NULL, in which case it does not do the work relating to gathering tty output/bzip2ing, otherwise it does and writes it to that file. Now if you look at https://github.com/facebookresearch/nle/blob/main/nle/nethack/nethack.py#L253 you see that the low-Python-level Nethack object has a way to be reset with a new file, or without a new file. In the latter case, the current file keeps being used (and is not closed or reopened). In theory, that makes it possible to put all the output of a given NLE instance into a single file.

However, that's not what the higher-level gym interface in base.py does. As you point out, it seems to ask for a new file for each episode anyway. In which case rate-limiting it is a great idea and can easily be implemented on that level.

[dmadeka]

Okay - that was my understanding initially and then I got confused! The None solution seems ideal then - will open a PR today!

        if self._episode % self._episode_save_cycle == 0:
            self.last_observation = self.env.reset(new_ttyrec, wizkit_items=wizkit_items)
        else:
            self.last_observation = self.env.reset(None, wizkit_items=wizkit_items)