Numerical stability of embedding kernels #22401
Conversation
pytorchbot
added
module: cuda
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mrshenli has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
ngimel
left a comment
ngimel
left a comment
There was a problem hiding this comment.
there is still partials_per_segment_offset device_vector, and direct access to its elements.
|
It was my impression that the The direct access to the two elements is unfortunately necessary. It need to know the size of |
|
device_vector with default allocator template argument https://github.com/pytorch/pytorch/pull/22401/files#diff-5e16509ef3a09789b80e5d16f8dc3062R248 won't use any specified policy, as there's no policy argument in the constructor. |
|
Make sense, thanks |
ngimel
left a comment
ngimel
left a comment
There was a problem hiding this comment.
approving, hope this will fix @chenyangyu1988 accuracy problems.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mrshenli has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mrshenli has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Address the issue raised in pytorch/pytorch#22377. The PR pytorch/pytorch#22016 introduces a temporary tensor of weights `grad_weight_per_segment` of the same dtype as the end result, which can be a problem when using `float16`. In this PR, it now use a `float32` temporary tensor when the input is `float16`. ngimel, can I get you to review? I think I have fixed the issues you have pointed out. Pull Request resolved: pytorch/pytorch#22401 Differential Revision: D16077319 Pulled By: mrshenli fbshipit-source-id: 7cfad7f40b4d41a244052baa2982ab51bbbd7309
Summary: Address the issue raised in pytorch#22377. The PR pytorch#22016 introduces a temporary tensor of weights `grad_weight_per_segment` of the same dtype as the end result, which can be a problem when using `float16`. In this PR, it now use a `float32` temporary tensor when the input is `float16`. ngimel, can I get you to review? I think I have fixed the issues you have pointed out. Pull Request resolved: pytorch#22401 Differential Revision: D16077319 Pulled By: mrshenli fbshipit-source-id: 7cfad7f40b4d41a244052baa2982ab51bbbd7309
7 participants
Address the issue raised in #22377.
The PR #22016 introduces a temporary tensor of weights
grad_weight_per_segmentof the same dtype as the end result, which can be a problem when usingfloat16.In this PR, it now use a
float32temporary tensor when the input isfloat16.@ngimel, can I get you to review? I think I have fixed the issues you have pointed out.