-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
fix pserver sub-blocks #11585
fix pserver sub-blocks #11585
Conversation
Should we remove these sub blocks from blk_list in the RunAsyncLoop method of listen_and_serv_op, like what we do in the RunSyncLoop method? |
May not, RunAsyncLoop use |
if not op.has_attr('sub_block'): | ||
return | ||
return -1 |
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.
Who uses the return value?
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.
PADDLE_ENFORCE_GE(num_blocks, 2, | ||
"server program should have at least 2 blocks"); | ||
|
||
std::vector<int> optimize_block_id_list; | ||
for (int blkid = 1; blkid < num_blocks; ++blkid) { | ||
if (std::find(prefetch_block_id_list.begin(), prefetch_block_id_list.end(), | ||
blkid) == prefetch_block_id_list.end()) { | ||
blkid) == prefetch_block_id_list.end() && |
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.
Is there a way to make it beautiful?
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, please follow the comment: #11585 (review)
@@ -101,13 +101,16 @@ void ListenAndServOp::RunSyncLoop( | |||
framework::Scope *recv_scope, | |||
const std::vector<int> &prefetch_block_id_list) const { | |||
size_t num_blocks = program->Size(); | |||
auto skip_sub_blks = Attr<std::vector<int>>("skip_sub_blks"); |
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.
Record only the start block id and end block id for listen_and_serv
op to run parallelly. Sub-blocks append to the end to make the program in a strict order.
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.
Another way is that we record the parallel block ids, the benefit is don't need insert
block to the specified idx, just append block to pserver_program and record the optimize block id.
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.
Reuse that deprecated optimize_block
attribute could be cool.
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.
Will do that.
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.
LGTM, thanks this should work.
…_blocks fix pserver sub-blocks
Fixed #11429