Cleanup transpiler and move weight decay and clip on pservers #11039
Conversation
| # limitations under the License. | ||
|
|
||
|
|
||
| def delete_ops(block, ops): |
There was a problem hiding this comment.
Can we move these functions into program.py ?
There was a problem hiding this comment.
This function was moved out from framework.py before because we have remove_op already, and this function is only used by distributed transpiler.
|
|
||
| LOOKUP_TABLE_TYPE = "lookup_table" | ||
| LOOKUP_TABLE_GRAD_TYPE = "lookup_table_grad" | ||
| OP_ROLE_VAR_ATTR_NAME = core.op_proto_and_checker_maker.kOpRoleVarAttrName() | ||
| RPC_OP_ROLE_ATTR_NAME = op_role_attr_name = core.op_proto_and_checker_maker.kOpRoleAttrName( |
There was a problem hiding this comment.
Could you help to delete the redundant codes?
RPC_OP_ROLE_ATTR_NAME = op_role_attr_name = core.op_proto_and_checker_maker.kOpRoleAttrName()
There was a problem hiding this comment.
This is not a redundant line, RPC_OP_ROLE_ATTR_NAME is used everywhere.
| def same_or_split_var(p_name, var_name): | ||
| return p_name == var_name or p_name.startswith(var_name + ".block") | ||
|
|
||
|
|
||
| def split_dense_variable(var_list, service_count, min_block_size=8192): | ||
| def split_variable(var_list, service_count, min_block_size=8192): |
There was a problem hiding this comment.
It looks we can also move split_variable into program_utils.py.
There was a problem hiding this comment.
It's not a "program util" actually, splitting variables is only transpiler specific.
| return True | ||
| return False | ||
|
|
||
| def _is_optimizer_op(self, op): |
There was a problem hiding this comment.
Maybe we can use op_role to determinate whether the op is a optimize one.
There was a problem hiding this comment.
ops like weight decay and cliping and other ops will marked as optimize role. The other function _is_opt_role_op will do this job.
| def _update_dist_lookup_table_vars(self, param_list, grad_list, | ||
| params_grads): | ||
| # update self.table_param_grad and self.trainer_side_table_grad_list | ||
| program = self.origin_program | ||
| if self.has_distributed_lookup_table: |
There was a problem hiding this comment.
Sorry, it's not this PR codes, but could you add a TODO comment here to make distribute lookup table as an independency transpiler?
Fix #7432