Argument t_max in PCL misused

[lyx-x]

According to the docstring below, t_max in PCL is equivalent to update_interval in DQN.

chainerrl/chainerrl/agents/pcl.py

Line 49 in 4ca100c

t_max (int): The model is updated after every t_max local steps

However, if t_max == None by default (also in the example script), line 388 (below), the update, will never be executed.

chainerrl/chainerrl/agents/pcl.py

Line 388 in 4b051a4

if self.t - self.t_start == self.t_max:

In addition, when sampling from the replay buffer, I think that max_len should be set with another parameter, at least not with t_max unless we change the docstring. (I also think there is no such limit on the length of episode in the original paper)

chainerrl/chainerrl/agents/pcl.py

Line 284 in 4b051a4

self.batchsize, max_len=self.t_max)

[muupan]

When t_max is set to None, PCL is expected to work as the original paper does, i.e. sampling a full episode.
When t_max is set to non-None value, it is expected to work like ACER with PCL's loss function.

It seems the docstring of PCL is wrong, not only in t_max but also in other points. We should fix this. Thank you for pointing it out.

[lyx-x]

My pull request is probably not what you expect, I added a new argument for limiting the length of episode. I should probably rename the argument.

By the way, the docstring of t_max in ACER may also be wrong.

chainerrl/chainerrl/agents/acer.py

Line 256 in 4b051a4

t_max (int): The model is updated after every t_max local steps