Codecov Report

Merging #261 (c4197e9) into master (cd48142) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #261      +/-   ##
==========================================
+ Coverage   94.40%   94.54%   +0.13%     
==========================================
  Files          41       41              
  Lines        2611     2677      +66     
==========================================
+ Hits         2465     2531      +66     
  Misses        146      146              

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd48142...c4197e9. Read the comment docs.

Seems it cannot work well with cuda tensor. (github action doesn't have cuda environment :(

$ python3 test/base/test_buffer.py                                               
Traceback (most recent call last):
  File "test/base/test_buffer.py", line 327, in <module>
    test_hdf5()
  File "test/base/test_buffer.py", line 300, in test_hdf5
    vbuf.save(path)
  File "/home/trinkle/github/tianshou-new/tianshou/data/buffer.py", line 409, in save
    self._copy_to_hdf5(k, v, f)
  File "/home/trinkle/github/tianshou-new/tianshou/data/buffer.py", line 370, in _copy_to_hdf5
    grp.create_dataset(k, data=v.numpy())
TypeError: can't convert cuda:0 device type tensor to numpy. Use Tensor.cpu() to copy the tensor to host memory first.

update: resolved

Also, could you please add this feature description in the docstring of ReplayBuffer?

It looks perfect to me ! Ready to be merged :) Very good job @nicoguertler

Great! Thank you @duburcqa and @Trinkle23897 for your help and advice! There is still some room for improvement. For example, there is no custom conversion for the SegmentTree so for now it is just pickled. But it should be easy to add that later on.

There is still some room for improvement. For example, there is no custom conversion for the SegmentTree so for now it is just pickled. But it should be easy to add that later on.

Yes, but the current implementation can be extended very easily, which is great !

Looks good to me now @Trinkle23897