search

NM512 / dreamerv3-torch

Implementation of Dreamer v3 in pytorch.
MIT License
425 stars 96 forks source link

Mutable Default Argument #50

Closed cdimmig closed 8 months ago

cdimmig commented 8 months ago

Hello!

The recursively_collect_optim_state_dict function has a mutable default argument. Due to the weird/magical/confusing ways that Python works it seems like the default argument itself gets updated causing all future calls to this function to have a pre-populated visited set.

Relevant stackoverflow link for this issue: https://stackoverflow.com/questions/1132941/least-astonishment-and-the-mutable-default-argument?fbclid=IwAR1pR_luIkKqRzeLoDBYC38hUt3Qxnc_3wtnZfLAbEoAsiaFNuM1mFiJNbI

https://github.com/NM512/dreamerv3-torch/blob/2c7a81a0e2f5f0c7659ba73b0ddbedf2a7e2ecf4/tools.py#L965-L967

I propose the following change to avoid this issue:

def recursively_collect_optim_state_dict(
    obj, path="", optimizers_state_dicts=None, visited=None
):
    if visited is None:
        visited = set()
NM512 commented 8 months ago

Hi there,

Thank you for bringing up this concern. I've made the necessary modification in this commit to address it. Thanks again for pointing this out!