-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 and optimize async distribute lookup table #10973
Conversation
@@ -731,6 +741,15 @@ def _clone_var(block, var, persistable=True): | |||
type="sum", | |||
inputs={"X": table_grad_list}, | |||
outputs={"Out": [grad_var]}) | |||
else: |
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.
In https://github.com/PaddlePaddle/Paddle/pull/10973/files#diff-3a23ecb6a98b666c84525f3f6fcd2bb6R729, table_grad_list
maybe confused with self. table_grad_list
, I think maybe better to store in different name, or seperate trainer side and server side transpile processes.
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.
follow comment, change to trainer_side_table_grad_list and pserver_side_table_grad_list
@@ -678,7 +688,7 @@ def _create_prefetch_block(self, pserver_index, pserver_program, | |||
return prefetch_block | |||
|
|||
def _create_table_optimize_block(self, pserver_index, pserver_program, | |||
pre_block_idx): | |||
pre_block_idx, grad_to_block_id): | |||
def _clone_var(block, var, persistable=True): |
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.
Can use block.clone_variable instead?
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.
good point, done
…qiao/Paddle into fix-prefetch
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!
project: #10868