-
Notifications
You must be signed in to change notification settings - Fork 3.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
[Relay] Improve numerical gradient check #3856
Conversation
@MarisaKirisame @vinx13 @junrushao1994 please review |
if inputs is None: | ||
params = fwd_func.params | ||
# Generate random inputs on the same scale as epsilon to avoid numerical precision loss. | ||
inputs = [_np_randn_from_type(x.checked_type, scale=(10 * eps)) for x in params] |
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 practice, I found that this heuristic works well for operations that involve multiplication/addition
For division you still get precision problems, especially when using float32. In those cases, either supply manually selected inputs, or use float64.
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, I verified that conv2d grad can pass numerical test now
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
I was having trouble getting passing tests with the existing gradient check implementation, even though the gradient implementations match results from the keras framework. This change rewrites the gradient check helper to perform two-sided numerical checks on every gradient dimension. I have an upcoming pull request with several gradient implementations that I was able to verify with this implementation.