Skip to content

Use Chainer v2 - #100

Merged
muupan merged 34 commits into
chainer:masterfrom
toslunar:chainerv2
Jun 7, 2017
Merged

Use Chainer v2#100
muupan merged 34 commits into
chainer:masterfrom
toslunar:chainerv2

Conversation

@toslunar

Copy link
Copy Markdown
Member

No description provided.

@coveralls

Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling a855613 on toslunar:chainerv2 into ** on pfnet:master**.

@coveralls

Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling f7e6a99 on toslunar:chainerv2 into ** on pfnet:master**.

@coveralls

coveralls commented May 30, 2017

Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 04ede2d on toslunar:chainerv2 into ** on pfnet:master**.

Comment thread chainerrl/agent.py
"""Load internal states."""
for attr in self.saved_attributes:
serializers.load_npz(
load_npz_no_strict(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the reason behind this change? It's better to add comments if this is a fix for some problem.

Comment thread chainerrl/agents/a3c.py Outdated
# Update the globally shared model
if self.process_idx == 0:
norm = self.optimizer.compute_grads_norm()
# norm = self.optimizer.compute_grads_norm()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove commented out code.

Comment thread chainerrl/agents/acer.py Outdated
# Update the globally shared model
if self.process_idx == 0:
norm = self.optimizer.compute_grads_norm()
# norm = self.optimizer.compute_grads_norm()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove commented out code.

Comment thread chainerrl/agents/pcl.py Outdated
target_link=self.shared_model, source_link=self.model)
if self.process_idx == 0:
norm = self.optimizer.compute_grads_norm()
# norm = self.optimizer.compute_grads_norm()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove commented out code.

Comment thread chainerrl/agents/pcl.py Outdated
self.v_loss_coef = v_loss_coef
self.rollout_len = rollout_len
self.batchsize = batchsize
self.batchsize = self.xp.int32(batchsize)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess this is a fix for newint, but it's not clear from the code. Can you add a comment?

class LeCunNormal(chainer.initializers.HeNormal):
"""sorry

"""

@muupan muupan Jun 2, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Elaborate the docstring.

Comment thread chainerrl/misc/async.py Outdated
def ensure_initialized_update_rule(param):
u = param.update_rule
if u.state is None:
u._state = {} # Sorry!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Eraborate the comment.

Comment thread .travis.yml Outdated
- cd chainer
- git checkout _v2
- python setup.py install
- cd ..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that chainer v2 is released, can you undo these changes?

Comment thread tests/misc_tests/test_async.py Outdated
opt_b = optimizers.RMSprop()
opt_b.setup(model)
opt_b.setup(copy.deepcopy(model))
opt_b.update() # sorry

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Elaborate the comment.

arrays = async.share_states_as_shared_arrays(opt_a)
opt_b = optimizers.RMSprop()
opt_b.setup(model)
opt_b.setup(copy.deepcopy(model))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is copy.deepcopy required here?

@muupan

muupan commented Jun 2, 2017

Copy link
Copy Markdown
Member

Great work! Have you checked tests (including gpu and slow tests) in your environment?

@toslunar

toslunar commented Jun 2, 2017

Copy link
Copy Markdown
Member Author

Some slow tests fail because of BLAS errors that seem to be chainer/chainer#2744.
I'm doing gpu tests.

@muupan

muupan commented Jun 7, 2017

Copy link
Copy Markdown
Member

LGTM

@muupan
muupan merged commit 6ccc61d into chainer:master Jun 7, 2017
@toslunar
toslunar deleted the chainerv2 branch June 7, 2017 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants