-
Notifications
You must be signed in to change notification settings - Fork 5.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
Clean not need ctx arguments. #10842
Conversation
auto var = var_h.scope->FindVar(var_h.name); | ||
|
||
platform::Place place = platform::CPUPlace(); | ||
if (var->IsType<framework::LoDTensor>()) { |
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.
Add a function to get the place?
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.
Done.
} else if (var->IsType<framework::SelectedRows>()) { | ||
place = var->Get<framework::SelectedRows>().place(); | ||
#ifdef PADDLE_WITH_CUDA | ||
} else if (var->IsType<ncclUniqueId>()) { |
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.
No return value for this branch.
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.
It uses the default value.
Done.
@@ -196,6 +189,8 @@ class RPCClient { | |||
grpc::CompletionQueue cq_; | |||
std::map<std::string, std::shared_ptr<grpc::Channel>> channels_; | |||
int64_t req_count_ = 0; | |||
|
|||
static const int64_t rpc_time_out = 600 * 1000; |
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.
rpc_time_out ==> kRpcTimeOut
https://google.github.io/styleguide/cppguide.html#Constant_Names
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.
Great.Done.
Since you haven't replied for a long time, we have closed this issue/pr. |
Fix part of #10804