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

More check on nesting AttributeSavingMixin #208

Merged
merged 3 commits into from Jan 29, 2018

Conversation

toslunar
Copy link
Member

This PR improves #134 so that save and load detect infinite loops of length >= 2.

@muupan
Copy link
Member

muupan commented Jan 29, 2018

Thank you and sorry for late response. Can you add a test case to check it can detect an infinite loop?

@toslunar
Copy link
Member Author

Added tests to detect loops.

I confirmed the current master fails the new test.

.E..
======================================================================
ERROR: test_loop2 (tests.test_agent.TestAttributeSavingMixin)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/tos/Documents/chainerrl/tests/test_agent.py", line 99, in test_loop2
    parent1.save(dirname)
  File "/Users/tos/Documents/chainerrl/chainerrl/agent.py", line 119, in save
    attr_value.save(os.path.join(dirname, attr))
  File "/Users/tos/Documents/chainerrl/chainerrl/agent.py", line 119, in save
    attr_value.save(os.path.join(dirname, attr))
  File "/Users/tos/Documents/chainerrl/chainerrl/agent.py", line 119, in save
    attr_value.save(os.path.join(dirname, attr))
  [Previous line repeated 154 more times]
  File "/Users/tos/Documents/chainerrl/chainerrl/agent.py", line 123, in save
    getattr(self, attr))
  File "/Users/tos/Documents/chainer/chainer/serializers/npz.py", line 73, in save_npz
    with open(file, 'wb') as f:
OSError: [Errno 63] File name too long: '/var/folders/zk/2tl6l77170d2b51fczxysjsw0000gn/T/tmpsawjvmga/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/child/link.npz'

----------------------------------------------------------------------
Ran 4 tests in 0.530s

FAILED (errors=1)

@muupan
Copy link
Member

muupan commented Jan 29, 2018

LGTM

@muupan muupan merged commit ab96d18 into chainer:master Jan 29, 2018
@toslunar toslunar deleted the recursive-save branch January 29, 2018 10:10
@muupan muupan added this to the v0.4 milestone Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants