Use faster queue for replay buffers

[toslunar]

TODO:

• Add tests
• Measure performance

This will happen to resolve #36 (see also #128)

[coveralls]

Coverage decreased (-0.3%) to 71.167% when pulling 34c80ab on toslunar:replace-deque into e095992 on chainer:master.

[coveralls]

Coverage increased (+0.005%) to 71.518% when pulling d7299fc on toslunar:replace-deque into e095992 on chainer:master.

[toslunar]

😰
before: 13.168848936009454
after: 16.709212244983064

import numpy as np
import timeit

from chainerrl import replay_buffer


def rand_state():
    return np.random.rand(1, 3, 50, 20).astype(np.float32)


def rand_action():
    return np.random.rand(1, 40).astype(np.float32)


def rand_reward():
    return np.random.rand()


def f(capacity, batch_size, steps, replay_start_size):
    rbuf = replay_buffer.ReplayBuffer(capacity=capacity)
    s = rand_state()
    a = rand_action()
    for i in range(steps):
        next_s = rand_state()
        next_a = rand_action()
        rbuf.append(s, a, rand_reward(), next_s, next_a)
        s = next_s
        a = next_a
        if i >= replay_start_size:
            rbuf.sample(batch_size)


print(min(timeit.Timer(
    'f(10000, 64, 100000, 1000)',
    setup="from __main__ import f; gc.enable()").
    repeat(repeat=3, number=1)))

[toslunar]

before: 337.00381314000697
after: 171.62339709801017
for f(100000, 64, 1000000, 10000).

[muupan]

Good job! But this solution seems unecessarily complex to me. How about using a ring buffer like this? https://github.com/matthiasplappert/keras-rl/blob/master/rl/memory.py#L35

I think we can assume maxlen is specified for replay buffers, otherwise training would face out-of-memory eventually.

[toslunar]

EpisodicReplayBuffer limits the number of the transitions in the buffer. It's not easy to determine the best maxlen of self.episodic_memory = RingBuffer(maxlen). Of course, there is sufficiently large one (and this doesn't cost memory much, relatively to other parts): self.episodic_memory = RingBuffer(capacity).

[toslunar]

• collections.deque + random.sample: 8.064196354011074

q = deque(maxlen=10000)
for _ in range(100000):
    q.append(1)
    if len(q) > 1000:
        random.sample(q, 64)

• RandomAccessQueue
  -- at d7299fc: 13.529830269049853
  -- at fc39b20: 10.133344302070327
  -- at b8bec91: 5.198489462025464

q = RandomAccessQueue(maxlen=10000)
for _ in range(100000):
    q.append(1)
    if len(q) > 1000:
        q._sample(64)

[coveralls]

Coverage increased (+0.02%) to 71.536% when pulling 88c9fac on toslunar:replace-deque into e095992 on chainer:master.

[coveralls]

Coverage increased (+0.02%) to 71.536% when pulling 88c9fac on toslunar:replace-deque into e095992 on chainer:master.

[coveralls]

Coverage increased (+0.01%) to 71.526% when pulling 874b4ad on toslunar:replace-deque into e095992 on chainer:master.

[coveralls]

Coverage increased (+0.1%) to 71.624% when pulling 312c025 on toslunar:replace-deque into e095992 on chainer:master.

[coveralls]

Coverage increased (+0.1%) to 71.624% when pulling fd14a7b on toslunar:replace-deque into e095992 on chainer:master.

[coveralls]

Coverage increased (+0.1%) to 71.624% when pulling 1d9b658 on toslunar:replace-deque into e095992 on chainer:master.

[toslunar]

time (secs) after the debug:

(steps, maxlen)	baseline: collections.deque + random.sample	collections.deque + _sample_n_k	RandomAccessQueue + _sample_n_k
(100000, 10000)	7.3263045670464635	5.774126871023327	7.128595568938181
(300000, 30000)	26.707710441900417	21.629545310977846	20.986778931925073
(1000000, 100000)		197.8464105380699	74.75397148309276

Non-repetitive sampling by [q[i] for i in _sample_n_k(len(q),64)] is faster than by random.sample(q, 64). With this faster sampling, RandomAccessQueue is faster than collections.deque when maxlen is larger than around 30000.

[muupan]

LGTM except two comments. Great work!

[muupan]

This method is called from ReplayBuffer, thus should be public.

Use one leading underscore only for non-public methods and instance variables.

https://www.python.org/dev/peps/pep-0008/#method-names-and-instance-variables

[muupan]

Although your implementation looks correct, scipy has scipy.stats.kstest and I think it's better to use the existing well-tested implementation.

You can get p-value by scipy.stats.kstest(xs, 'norm', args=(mean, std))[1].

[muupan]

Add future imports (at least range is affected)

[okuta]

Fixed.

[toslunar]

Fixed.

[coveralls]

Coverage increased (+0.2%) to 71.665% when pulling eba0baa on toslunar:replace-deque into e095992 on chainer:master.

[coveralls]

Coverage increased (+0.2%) to 71.665% when pulling eba0baa on toslunar:replace-deque into e095992 on chainer:master.