Closed maciejors closed 10 months ago
Yes I agree, It can be instance attribute with just default values same, as this ones already set. I can fix this and do the way you already mentioned.
In my opinion none of the currently static attributes should be static
I've raised this issue in the group chat a while ago and I was explained that they should remain static but I'm still not fully convinced about at least some of the attributes.
One of these is learning rate which is an instance attribute for
PPOAgent
but a static attribute forDQNAgent
. This makes no sense for me since a user might want to configure that per agent (as I did for PPO in experiment 4). Same I believe (judging from their documentation) is the case for DQN'sUPDATE_EVERY
and PPO'sn_episodes
/n_steps
.From what I know from @Szymon-Gut other static attributes are rarely changed and therefore is not a big deal to leave them static. However I just don't see much of a reason not to make them instance attributes. What if a user for whatever reason wants to configure them for each agent individually? Is there any specific reason why we want to restrict that? If not, I'd opt for making these instance attributes and adding them to the constructor.