-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Refine P2PSync #3588
Refine P2PSync #3588
Conversation
Please squash the lint fixes -- we like to keep the history clear. |
ba533e1
to
130c05a
Compare
@@ -94,6 +94,9 @@ class P2PSync : public GPUParams<Dtype>, public Solver<Dtype>::Callback, | |||
} | |||
|
|||
void run(const vector<int>& gpus); | |||
void prepare(const vector<int>& gpus, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be Prepare
according to the style guide and existing style. Since we already have run
, however, I guess we can postpone that to a future commit.
Thanks, this patch looks good except for the getter name issue above, an optionally fix up existing naming issues (member functions that do nontrivial work should be capitalized CamelCase.) |
Thanks for the review. We have revised the code accordingly. |
It looks there is a problem with travis-CI PYTHON-3.0 builds. It's not a problem with this PR. I will try to build again once that is fixed. |
Okay, this looks good except for the (more subtle) point in the comment above; feel free to adjust accordingly and rebase on master which should fix the Travis issue. |
Thanks for the patch, the diff and history look clean now, merging. |
Refine P2PSync
This PR makes the following changes into P2PSync class.