Fix sparse vars usage for dist train #11248
Conversation
panyx0718
requested review from
gongweibao and
typhoonzero
and removed request for
typhoonzero
typhoonzero
requested review from
Yancey0623
and removed request for
gongweibao and
typhoonzero
| // mini-batch. | ||
| // TODO(Yancey1989): move the reset action into an operator, we couldn't | ||
| // have any hide logic in the operator. | ||
| for (framework::Variable *var : sparse_vars) { |
There was a problem hiding this comment.
I think this is needed by sparse updates? @Yancey1989
There was a problem hiding this comment.
Yes, sparse_vars is need by remote sparse update, we need to clear it after each mini-bach, because not each sparse gradient var would been send to the pserver, the clear operation would avoid reuse the old var leaved from pre-mini-batch.
There was a problem hiding this comment.
sparse_vars seems always empty because it's not mutated?
There was a problem hiding this comment.
It would been update if the received var type is SelectedRows
Paddle/paddle/fluid/operators/detail/request_handler_impl.cc
Lines 67 to 70 in ff9b1a0
There was a problem hiding this comment.
sparse_vars_ is exposed by sparse_vars(), but it seems not called by anyone?
There was a problem hiding this comment.
Got it, you're right, seems it's a bug, we need to iterator sparse_vars_ instead of sparse_var which defined in listen_and_serv_op.
polish sparse update code
panyx0718
changed the title
|
this feature should be covered by our ut in the future |
3 participants
No description provided.