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.

Sign up for GitHub

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

Jump to bottom

Fix some options of MNIST example not working #3500

Merged
merged 3 commits into from
Feb 22, 2018
Merged

Fix some options of MNIST example not working #3500

merged 3 commits into from
Feb 22, 2018

Conversation

kmaehashi
Copy link
Member

No description provided.

@kmaehashi kmaehashi added the cat:example Example, e.g. the MNIST example. label Oct 26, 2017
beam2d
beam2d requested changes Oct 26, 2017
View reviewed changes
Copy link
Member

@beam2d beam2d left a comment

Choose a reason for hiding this comment

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

LGTM except for one comment.

examples/mnist/train_mnist_custom_loop.py Outdated
@@ -51,6 +51,10 @@ def main():
optimizer = chainer.optimizers.Adam()
optimizer.setup(model)

if args.resume:
# Resume from a snapshot
serializers.load_npz(args.resume, model)
Copy link
Member

Choose a reason for hiding this comment

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

Should optimizer be loaded as well?

@beam2d beam2d added the st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. label Oct 26, 2017
@kmaehashi kmaehashi assigned kmaehashi and hvy and unassigned kmaehashi Dec 7, 2017
@hvy
Copy link
Member

hvy commented Dec 7, 2017

@kmaehashi thanks for this fix. I assume that args.resume should be the directory corresponding to args.out. File names should be appended in the code when loading the snapshots (mlp.model, mlp.state).

@hvy
Copy link
Member

hvy commented Feb 19, 2018

Ping @kmaehashi

@kmaehashi
Copy link
Member Author

Sorry for the late response and thanks for the comment. I fixed as suggested.

@kmaehashi kmaehashi removed the st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. label Feb 19, 2018
@hvy
Copy link
Member

hvy commented Feb 20, 2018

jenkins, test this please.

@hvy
Copy link
Member

hvy commented Feb 20, 2018

LGTM, @beam2d could you approve?

@hvy
Copy link
Member

hvy commented Feb 22, 2018

jenkins, test this please.

@hvy
Copy link
Member

hvy commented Feb 22, 2018

LGTM!

@hvy hvy added this to the v4.0.0rc1 milestone Feb 22, 2018
@hvy hvy merged commit f1dc3b1 into chainer:master Feb 22, 2018
@hvy
Copy link
Member

hvy commented Feb 22, 2018

@kmaehashi I think this fix should be backported. What do you think?

@kmaehashi kmaehashi deleted the fix-mnist-custom-example branch February 22, 2018 04:08
@kmaehashi
Copy link
Member Author

@hvy I agree to backport it.

@kmaehashi kmaehashi added the to-be-backported Pull request that should be backported. label Feb 22, 2018
hvy added a commit to hvy/chainer that referenced this pull request Feb 22, 2018
@hvy
Merge pull request chainer#3500 from kmaehashi/fix-mnist-custom-example
9c85da4 
Fix some options of MNIST example not working
@hvy hvy mentioned this pull request Feb 22, 2018
Merged
@hvy
Copy link
Member

hvy commented Feb 22, 2018

@kmaehashi Thanks for the label, I created a backport.

@hvy hvy mentioned this pull request Feb 28, 2019
Merged
@crcrpar crcrpar mentioned this pull request Mar 1, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beam2d beam2d beam2d approved these changes

Assignees

@hvy hvy

Labels
cat:example Example, e.g. the MNIST example. to-be-backported Pull request that should be backported.
Projects
None yet
Milestone
v4.0.0rc1
Development

Successfully merging this pull request may close these issues.
3 participants
@kmaehashi @hvy @beam2d