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

Support ChainerX in CIFAR example #6936

Merged
merged 3 commits into from Apr 17, 2019
Merged

Support ChainerX in CIFAR example #6936

merged 3 commits into from Apr 17, 2019

Conversation

asi1024
Copy link
Member

@asi1024 asi1024 commented Apr 15, 2019

Part of #6661

@asi1024 asi1024 added cat:example Example, e.g. the MNIST example. to-be-backported Pull request that should be backported. ChainerX Related to ChainerX. labels Apr 15, 2019
@asi1024 asi1024 changed the title [WIP] Support ChainerX in CIFAR example Support ChainerX in CIFAR example Apr 15, 2019
@asi1024 asi1024 marked this pull request as ready for review April 15, 2019 19:07
@asi1024 asi1024 added this to Ready to Review in ChainerX sprint backlog via automation Apr 15, 2019
@niboshi niboshi moved this from Ready to Review to Reviewing in ChainerX sprint backlog Apr 16, 2019
niboshi
niboshi reviewed Apr 16, 2019
View reviewed changes
examples/cifar/README.md Outdated
```

TODO: Fix it.
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? Maybe you forgot to remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I remove this line.

examples/cifar/train_cifar_custom_loop.py
@@ -102,9 +109,7 @@ def main():
optimizer.lr *= 0.5
print('Reducing learning rate to: {}'.format(optimizer.lr))

x_array, t_array = convert.concat_examples(batch, args.gpu)
x = chainer.Variable(x_array)
t = chainer.Variable(t_array)
Copy link
Member

Choose a reason for hiding this comment

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

Just to be clear, this is just a nonessential code improvement, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. (Actually, I thought that it is necessary changes, but I found it unnecessary after I send this PR.)

@niboshi niboshi moved this from Reviewing to Needs Changes in ChainerX sprint backlog Apr 16, 2019
@asi1024 asi1024 moved this from Needs Changes to Ready to Review in ChainerX sprint backlog Apr 16, 2019
@niboshi
Copy link
Member

niboshi commented Apr 16, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit eff764a (769056a):

@niboshi niboshi added the st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. label Apr 16, 2019
@niboshi niboshi moved this from Ready to Review to Waiting for CI in ChainerX sprint backlog Apr 16, 2019
@niboshi
Copy link
Member

niboshi commented Apr 17, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit eff764a (769056a):

@chainer-ci
Copy link
Member

Jenkins CI test (for commit eff764a, target branch master) failed with status FAILURE.

@niboshi
Copy link
Member

niboshi commented Apr 17, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit eff764a (70ffdbb):

@chainer-ci
Copy link
Member

Jenkins CI test (for commit eff764a, target branch master) failed with status FAILURE.

@niboshi
Copy link
Member

niboshi commented Apr 17, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit eff764a (cb8748b):

@chainer-ci
Copy link
Member

Jenkins CI test (for commit eff764a, target branch master) failed with status FAILURE.

@niboshi
Copy link
Member

niboshi commented Apr 17, 2019

LGTM

@niboshi niboshi merged commit 718c573 into chainer:master Apr 17, 2019
ChainerX sprint backlog automation moved this from Waiting for CI to Done Apr 17, 2019
niboshi added a commit to niboshi/chainer that referenced this pull request Apr 17, 2019
@niboshi
Merge pull request chainer#6936 from asi1024/chx-cifar-example
774620a 
Support ChainerX in CIFAR example
@niboshi niboshi mentioned this pull request Apr 17, 2019
Merged
@asi1024 asi1024 deleted the chx-cifar-example branch April 17, 2019 07:47
@asi1024 asi1024 added this to the v7.0.0a1 milestone Apr 17, 2019
@niboshi niboshi self-assigned this Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niboshi niboshi niboshi left review comments

Assignees

@niboshi niboshi

Labels
cat:example Example, e.g. the MNIST example. ChainerX Related to ChainerX. st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. to-be-backported Pull request that should be backported.
Projects
ChainerX sprint backlog
  
Done
Milestone
v7.0.0a1
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@asi1024 @niboshi @pfn-ci-bot @chainer-ci