eleurent / rl-agents

Implementations of Reinforcement Learning and Planning algorithms
MIT License
591 stars 153 forks source link

Fix: finalised monitor deprecation in experiments and evaluation scripts #82

Closed TibiGG closed 2 years ago

TibiGG commented 2 years ago

I was about to do some performance improvement modifications for the DQN runner, hoping some stuff could be up-streamable, when discovering that some monitor references were left in place, even after the deprecation, in some quite important places as well (like the experiments script).

I hope I got it right that the monitor stats recorder was replaced by the wrapped environment. If not, pls give me a script I could use to debug and test the problem better. I personally only need to use the experiments.py script.

Otherwise, the only other references to the monitor are comments, which I would rather not tackle at this point in time. If you have suggestions, pls mention them, so I can add them in.

TibiGG commented 2 years ago

@eleurent, please let me know if you have any comments for this pull request

eleurent commented 2 years ago

Hi, sorry for the late reply, I am currently on vacation. I remember that when I originally looked at this PR I just wasn't sure about the use of wrapped_env, but had no time to investigate. I just checked and it actually seems correct (I should really rename this field instead, as this now refers to the wrapper and not the wrapped env anymore).

eleurent commented 2 years ago

And sorry you had to go through that, I rushed this deprecation of Monitor and it was quite sloppy.