-
Notifications
You must be signed in to change notification settings - Fork 335
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] Check None value for Pytorch model gradients histogram #1917
Conversation
@kage08 thanks a lot for the contribution! 🙌 |
@kage08 thanks a lot for the contribution. 🙏 |
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.
@kage08 thanks for contribution 🙌
may I ask you to update the CHANGELOG.md to include the fix in Unreleased
section?
also I've changed the PR header to match the naming conventions.
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.
looks good!
the remaining steps:
- open a GH issue
- update CHANGELOG.md to include this fix
Made a minor fix in the changelog :) btw, it is worth considering getting the weight tensor only once, isn't it? e.g: weight = None
if hasattr(m, 'weight') and m.weight is not None:
weight = getattr(m.weight, dt, None)
if weight is not None:
layers[layer_name]['weight'] = get_pt_tensor(getattr(m.weight, dt)).numpy() instead of: if hasattr(m, 'weight') and m.weight is not None and getattr(m.weight, dt, None) is not None:
layers[layer_name]['weight'] = get_pt_tensor(getattr(m.weight, dt)).numpy() thoughts? @kage08 @alberttorosyan |
This makes sense for me. It avoids getting value twice. |
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.
@kage08 looks awesome 🙌
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.
@kage08 looks good 🙌 thanks a lot for contribution
Merged the PR! @kage08 congrats on the first merged contribution! 🚀 |
When tracking histograms of gradients, some tensors may not have gradients during specific training loops.
So check if grad is not None before converting to numpy.
This commit fixes #1925