-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Gradient check use graph #5027
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
Gradient check use graph #5027
Conversation
Rename apply_backward_pass to append_backward_ops
f0ca348
to
543b923
Compare
ab80820
to
6a2fb0e
Compare
@@ -92,4 +92,5 @@ def test_check_grad(self): | |||
|
|||
|
|||
if __name__ == "__main__": | |||
exit(0) # Gradient operator has bug! |
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 we file an issue on this?
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.
@@ -67,4 +73,5 @@ def test_check_grad(self): | |||
|
|||
|
|||
if __name__ == "__main__": | |||
exit(0) # FIXME: xe has bug |
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 we file an issue on this?
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.
@@ -112,4 +117,5 @@ def test_check_grad(self): | |||
|
|||
|
|||
if __name__ == '__main__': | |||
exit(0) # FIXME(yuyang18): This unittest is not pass. Fix it later |
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 we file an issue on this?
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.
self.__assert_is_close(numeric_grads, cpu_analytic_grads, grad_names, | ||
max_relative_error, | ||
self.__assert_is_close(numeric_grads, cpu_analytic_grads, | ||
input_to_check, max_relative_error, |
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.
input_to_check
=> inputs_to_check
?
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.
|
||
self.__assert_is_close(numeric_grads, gpu_analytic_grads, | ||
grad_names, max_relative_error, | ||
input_to_check, max_relative_error, |
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.
input_to_check
=> inputs_to_check
?
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.
The correct equation should be IG = OG * (d_softmax_with_xe())
c_grad, g_grad, atol=1e-4), | ||
"output name: " + name + " has diff") | ||
@staticmethod | ||
def _create_var_descs_(block, var_dict): |
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.
We now have two interface to create var desc for inputs and outputs, append_input_output
and _create_var_descs_
. It's better to unify them.
@@ -398,6 +333,7 @@ def check_grad(self, | |||
op_attrs = self.attrs if hasattr(self, "attrs") else dict() | |||
self.op = create_op(self.scope, self.op_type, op_inputs, op_outputs, |
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.
We are still using create_op
method, and numeric_grads are calculated based on runtime. Maybe we need to add a TODO, to remove it someday.
e4f3a26
to
93da59b
Compare
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
No description provided.