-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Migrate erfinv and erfinv_ from the TH to Aten (CUDA) #24943
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
Conversation
| }); | ||
| } | ||
|
|
||
| static void erfinv_kernel(TensorIterator& iter) { |
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.
I had to implement this method to be able to use dispatch. It will implemented when merging with correspondent CPU PR: #24908
| } | ||
|
|
||
| void erfinv_kernel_cuda(TensorIterator& iter) { | ||
| AT_DISPATCH_FLOATING_TYPES_AND_HALF(iter.dtype(), "erfinv_cuda", [&]() { |
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.
_AND_HALF?
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.
Currently it's generated for FLOAT, DOUBLE and HALF
|
@pytorchbot retest this please |
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.
@ifedan has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| /*check_mem_overlap=*/true); \ | ||
| op##_stub(iter.device_type(), iter); \ | ||
| return result; \ | ||
| #define IMPLEMENT_UNARY_OP_CORE(op) \ |
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.
There are many reasons not to use macro (terrible debug experience). Please check #24879 if solution is suitable for you.
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.
Overall good,, but I rather wait #24879 to land and use helpers from it.
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.
@ifedan has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: pytorch/pytorch#24560 Pull Request resolved: pytorch/pytorch#24943 Differential Revision: D16996434 Pulled By: ifedan fbshipit-source-id: 77111a4e47bb2b20f65225d48e7213cd77ddae19
…ble (#25337) Summary: This best preserves accuracy, while erfinvf() should be used for half and float. This is also consistent with the implementation before the migration: #24943 Pull Request resolved: #25337 Differential Revision: D17102333 Pulled By: zou3519 fbshipit-source-id: 5178cff534cf5f10d86ab04d4b6c1779ffedf49e
…ble (#25337) Summary: This best preserves accuracy, while erfinvf() should be used for half and float. This is also consistent with the implementation before the migration: pytorch/pytorch#24943 Pull Request resolved: pytorch/pytorch#25337 Differential Revision: D17102333 Pulled By: zou3519 fbshipit-source-id: 5178cff534cf5f10d86ab04d4b6c1779ffedf49e

#24560