Add C++ API clip_grad_value_ for nn:utils #28736
Conversation
jokerkeny
requested review from
ebetica,
goldsborough and
yf225
as code owners
|
Hi, I wonder how to create an undefined tensor in C++ PyTorch as None in Python. As test_nn.py has: for grad_list in [[grad_w, grad_b], [grad_w, None]]:
for p, g in zip(l.parameters(), grad_list):
p._grad = g.clone().view_as(p.data) if g is not None else gI think the best way to check None in C++ is by |
yf225
left a comment
yf225
left a comment
There was a problem hiding this comment.
@jokerkeny Thanks so much for the awesome work! I left some minor comments.
| clip_grad_value_(l.parameters(), clip_value) | ||
| for p in filter(lambda p: p.grad is not None, l.parameters()): | ||
| self.assertLessEqual(p.grad.data.max(), clip_value) | ||
| self.assertGreaterEqual(p.grad.data.min(), -clip_value) |
There was a problem hiding this comment.
Curious what's the reason we need to indent these lines?
There was a problem hiding this comment.
Hi, I think they should be wrapped under for grad_list in [[grad_w, grad_b], [grad_w, None]]:
The original code is:
for grad_list in [[grad_w, grad_b], [grad_w, None]]:
for p, g in zip(l.parameters(), grad_list):
p._grad = g.clone().view_as(p.data) if g is not None else g
clip_grad_value_(l.parameters(), clip_value)
for p in filter(lambda p: p.grad is not None, l.parameters()):
self.assertLessEqual(p.grad.data.max(), clip_value)
self.assertGreaterEqual(p.grad.data.min(), -clip_value)I checked that, only [grad_w, None] would be test.
There was a problem hiding this comment.
ah I see, thanks so much for catching it! :D
yf225
left a comment
yf225
left a comment
There was a problem hiding this comment.
@jokerkeny Thanks so much for the awesome work!
| clip_grad_value_(l.parameters(), clip_value) | ||
| for p in filter(lambda p: p.grad is not None, l.parameters()): | ||
| self.assertLessEqual(p.grad.data.max(), clip_value) | ||
| self.assertGreaterEqual(p.grad.data.min(), -clip_value) |
There was a problem hiding this comment.
ah I see, thanks so much for catching it! :D
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@yf225 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
4 participants
Summary:
Adds C++ API clip_grad_value_ for torch::nn:utils module.
Also, fix the for indent level error in the original test/test_nn.py.
Issue: #25883
Reviewer: @yf225