Fix episodic buffer __len__ #155
Conversation
The behaviour of episodic replay buffers has been changed so that now __len__ returns the number of transitions, not episodes, to fix chainer#138. You can use the n_episodes property to get the number of episodes in the buffer.
chainerrl/replay_buffer.py
Outdated
| def stop_current_episode(self): | ||
| """Notify the buffer that the current episode is interrupted. | ||
|
|
||
| When a transtion with is_state_terminal=True is appended, the buffer |
There was a problem hiding this comment.
I suggest starting the document with the cases when the method should be called, for example:
You may want to interrupt the current episode and start a new one before observing a terminal state. This is typical in continuing envs. In such cases, you need to call this method before appending a new transition so that the buffer will treat it as an initial transition of a new episode.
This method should not be called after an episode whose termination is already notified by appending a transition with is_state_terminal=True.
chainerrl/replay_buffer.py
Outdated
| @@ -14,14 +14,16 @@ | |||
| from chainerrl.misc.prioritized import PrioritizedBuffer | |||
|
|
|||
|
|
|||
| class ReplayBuffer(object): | |||
| class AbstractReplayBuffer(object): | |||
There was a problem hiding this comment.
Why don't you decorate the methods with @abstractmethod?
and raise an error instead of pass
|
Thank you for your review! I added a check of |
|
LGTM! |
AbstractReplayBufferandAbstractEpisodicReplayBufferto clarify the interfaces of replay buffers__len__ofEpisodicReplayBufferandPrioritizedEpisodicReplayBufferso that they now return the number of transitions, not of episodes.n_episodesproperty to count the number of episodesResolves #138