-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Migrated hardshrink() to ATen and deprecated nn.Hardshrink() #8117
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
…; 3. reusing previous tests for nn.Hardshrink() and included CUDA tests at test_nn; 4. default parameter lambda=0.5 is not working yet
|
|
||
| Tensor hardshrink_cuda(const Tensor & self, Scalar lambd) { | ||
| auto lambd_tensor = at::zeros_like(self).fill_(lambd); | ||
| auto out_tensor = self.clone(); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| module.half().cuda() | ||
| module(input) | ||
| for o in module.parameters(): | ||
| for p in module.parameters(): |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
The default parameter lambda=0.5 is still not working. Currently I declared it at |
|
Default should work. Can you push with the test uncommented so we can see the failure? Re deprecation, cc @soumith and @colesbury, but it won't be easy to "deprecate" the |
|
Now the failure should appears: The 3rd lambd equals 0, where it should be default value 0.5 |
|
For reasons I don't understand, this PR also allows nn.Hardshrink() to work in both of CPU and CUDA. For instance: Is nn.Hardshrink() borrowing the implementation from torch.hardshrink()? Did I miss something? |
That wouldn't surprise me! Have you checked the code? |
|
nn.Hardshrink is using def from functional.py, and you've told functional.py to call torch.hardshrink. All is good, I think. |
…tive_functions.yaml, and declare it at nn/functional.py
|
@pytorchbot retest this please |
|
@pytorchbot retest this please |
|
caffe CI test failures look not related, I will merge this PR |
| } | ||
|
|
||
| Tensor hardshrink_backward_cpu(const Tensor & grad, const Tensor & self, Scalar lambd) { | ||
| auto lambd_tensor = lambd.toTensor().toType(self.type().scalarType()).toBackend(self.is_cuda() ? Backend::CUDA : Backend::CPU); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Summary:
This replaces #7695, fixes #4154
Future work:
Performance:
CPU forward:
Previous impl:
Current impl:
CPU backward:
Previous impl:
Current impl:
CUDA:
Current impl:
Benchmark: