Skip to content

Adam operator optimized with Eigen #10229

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

Merged
merged 7 commits into from
Apr 27, 2018
Merged

Adam operator optimized with Eigen #10229

merged 7 commits into from
Apr 27, 2018

Conversation

tpatejko
Copy link

This PR implements Adam operator optimized with Eigen. This is important for CPU execution.

Two benchmarks were taken into account: mnist and machine_translation. The results of profiling on default parameters, on Skylake CPU:

  • machine translation

    • plain: Adam op 25.5546ms, total time 315.52181 sec, 198.64237 examples/sed
    • Eigen: Adam op 4.82581, total time: 273.15408, 229.09049 examples/sed
  • mnist:

    • plain: Adam op 0.0897811, total time: 1.44365, 2659.92448 examples/sed
    • Eigen: Adam op 0.0146323, total time: 1.40162, 2739.67732 examples/sed

@tpatejko tpatejko requested a review from luotao1 April 26, 2018 10:55
@tpatejko
Copy link
Author

@luotao1 Could you have a look at this PR?

@tpatejko tpatejko added the Intel label Apr 26, 2018
Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks very much!

@luotao1 luotao1 merged commit e498e1f into PaddlePaddle:develop Apr 27, 2018
@luotao1
Copy link
Contributor

luotao1 commented Apr 27, 2018

@dzhwinter How about the same Eigen optimization on GPU execution?

param_out_(param_out) {}

void operator()(size_t numel) const {
Eigen::Map<const Eigen::Array<T, 1, Eigen::Dynamic>> g{
Copy link
Contributor

Choose a reason for hiding this comment

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

@tpatejko You can use framework::EigenTensor<T, 1> to replace Eigen::Map<const Eigen::Array<T, 1, Eigen::Dynamic>> in next PR.
We have a eigen wrap in https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/framework/eigen.h.

Copy link
Author

Choose a reason for hiding this comment

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

@luotao1 Thanks for the remark. I will use it next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants