Skip to content

[Caffe2][Training performance]Implementation MomentumSGD/MomentumSGDUpdate operators for mkl-dnn #11686

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
Closed
PenghuiCheng wants to merge 1 commit into from
Closed

[Caffe2][Training performance]Implementation MomentumSGD/MomentumSGDUpdate operators for mkl-dnn #11686

PenghuiCheng wants to merge 1 commit into from

Conversation

@PenghuiCheng
Copy link
Contributor

@PenghuiCheng PenghuiCheng commented Sep 14, 2018

the speed-up of a single operation is up to 6X on BDW.

@PenghuiCheng
Copy link
Contributor Author

PenghuiCheng commented Sep 14, 2018

@wesolwsk,Please review the commit.
Thanks!
cheng,Penghui

facebook-github-bot
facebook-github-bot reviewed Sep 14, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

wesolwsk has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@wesolwsk
Copy link
Contributor

wesolwsk commented Sep 14, 2018

Getting lots of errors as follows:

caffe2/caffe2/core/operator.h: In instantiation of ‘const T& caffe2::OperatorBase::Input(int) [with T = caffe2::Tensor]’:
caffe2/caffe2/ideep/operators/momentum_sgd_op.cc:55:55: required from here
caffe2/caffe2/core/operator.h:68:5: error: static assertion failed: You should use Input(int, DeviceType) for Tensor.
static_assert(
^

@PenghuiCheng
Copy link
Contributor Author

PenghuiCheng commented Sep 14, 2018

@wesolwsk ,I had updated code. Please review it.
Thanks!
cheng,Penghui

facebook-github-bot
facebook-github-bot reviewed Sep 18, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

wesolwsk has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@wesolwsk
Copy link
Contributor

wesolwsk commented Sep 19, 2018

please make the following fixes and rebase

screen shot 2018-09-19 at 2 12 28 pm

@wesolwsk
Copy link
Contributor

wesolwsk commented Sep 21, 2018

^ @PenghuiCheng

facebook-github-bot
facebook-github-bot reviewed Sep 24, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

wesolwsk has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@PenghuiCheng
Copy link
Contributor Author

PenghuiCheng commented Sep 25, 2018

@wesolwsk ,for line 37 and line78,ShadowingClass warning,I didn't know how to change it,do you have some suggestions for this?
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

Assignees

No one assigned

Labels

caffe2 open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@PenghuiCheng @wesolwsk @facebook-github-bot @ezyang @pytorchbot