Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix an error related to collation and RNN #129

Merged
merged 4 commits into from
Mar 3, 2021

Conversation

tarokiritani
Copy link
Contributor

Recurrent policies expect data collated in a tuple.

Recurrent policies expect data collated in a tuple.
@muupan muupan requested review from muupan and removed request for prabhatnagarajan February 10, 2021 01:08
Copy link
Member

@muupan muupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pfrl/utils/batch_states.py Outdated Show resolved Hide resolved
tarokiritani and others added 2 commits February 24, 2021 12:31
Omitted skipping.
The input feature type is checked better way.

Co-authored-by: Yasuhiro Fujita <muupan@gmail.com>
@muupan
Copy link
Member

muupan commented Mar 1, 2021

/test

@pfn-ci-bot
Copy link

Successfully created a job for commit 671b6d6:

@muupan
Copy link
Member

muupan commented Mar 2, 2021

It seems the test fails with gpu. Can you apply this patch so it will work with both cpu and gpu? Looks good except this failure.

diff --git a/tests/utils_tests/test_batch_states.py b/tests/utils_tests/test_batch_states.py
index ba4391b3..cdfedcd8 100644
--- a/tests/utils_tests/test_batch_states.py
+++ b/tests/utils_tests/test_batch_states.py
@@ -27,7 +27,7 @@ class TestBatchStates(unittest.TestCase):
         self.assertIsInstance(batch, tuple)
         batch_a, batch_b, batch_c = batch
         np.testing.assert_allclose(
-            batch_a,
+            batch_a.cpu(),
             np.asarray(
                 [
                     [[0, 2], [4, 6]],
@@ -35,9 +35,9 @@ class TestBatchStates(unittest.TestCase):
                 ]
             ),
         )
-        np.testing.assert_allclose(batch_b, np.asarray([0, 1]))
+        np.testing.assert_allclose(batch_b.cpu(), np.asarray([0, 1]))
         np.testing.assert_allclose(
-            batch_c,
+            batch_c.cpu(),
             np.asarray(
                 [
                     [0],

test_gpu() now correctly uses a GPU.
@muupan
Copy link
Member

muupan commented Mar 3, 2021

/test

@pfn-ci-bot
Copy link

Successfully created a job for commit b5f6907:

Copy link
Member

@muupan muupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@muupan muupan merged commit 5e49276 into pfnet:master Mar 3, 2021
@muupan muupan added this to the v0.3.0 milestone Jul 7, 2021
@muupan muupan added the bug Something isn't working label Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants