Skip to content

Conversation

@weiyangfb
Copy link
Contributor

@weiyangfb weiyangfb commented Jun 4, 2018

Summary:

  1. Added hardshrink() to ATen (CPU + GPU)
  2. Removed nn.Hardshrink(), but keeping file "aten/src/THNN/generic/HardShrink.c"
  3. Reusing nn.Hardshrink() tests in test_nn and including CUDA tests. Not sure test_nn is a good place for hardshrink() since it is no longer under nn
  4. Added tests in test_torch

This replaces #7695, fixes #4154

Future work:

  1. not supporting default lambd=0.5 in torch.hardshrink() due to a Scalar bug float default values for Scalar in native_functions.yaml don't work #8484

Performance:

CPU forward:

Previous impl:

large_data = torch.zeros(1000, 1000).fill_(0.3).requires_grad_()
def origfn(data):
  f = torch.nn.Hardshrink(0.3)
  large_out = f(data)
%timeit origfn(large_data) 
####################
100 loops, best of 3: 2.15 ms per loop

Current impl:

large_data = torch.zeros(1000, 1000).fill_(0.3).requires_grad_()
def newfn(data):
    large_out = data.hardshrink(0.3)
%timeit newfn(large_data)
####################
100 loops, best of 3: 2.62 ms per loop

CPU backward:

Previous impl:

large_data = torch.zeros(1000, 1000).fill_(0.3).requires_grad_()
def origfn_backward(data):
    f = torch.nn.Hardshrink(0.3)
    large_out = f(data)
    large_out.sum().backward()
%timeit origfn_backward(large_data)
####################
100 loops, best of 3: 5.2 ms per loop

Current impl:

large_data = torch.zeros(1000, 1000).fill_(0.3).requires_grad_()
def newfn_backward(data):
    large_out = data.hardshrink(0.3)
    large_out.sum().backward()
%timeit newfn_backward(large_data)
####################
100 loops, best of 3: 6.47 ms per loop

CUDA:

Current impl:

large_data = torch.zeros(1000, 1000, device=cuda).fill_(0.3)
%timeit data.hardshrink(0.3)
####################
10000 loops, best of 3: 97.1 µs per loop

Benchmark:

large_data = torch.zeros(1000, 1000, device=cuda).fill_(0.3)
%timeit data.mul_(2)
####################
10000 loops, best of 3: 51.6 µs per loop

…; 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.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

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.

@weiyangfb
Copy link
Contributor Author

weiyangfb commented Jun 6, 2018

The default parameter lambda=0.5 is still not working. Currently I declared it at native_functions.yaml, maybe more work is needed? Also, where should I add warning messages for the deprecation of nn.Hardshrink? @ezyang

@ezyang
Copy link
Contributor

ezyang commented Jun 7, 2018

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 legacy_nn codepath in a reasonable way. Maybe best to drop it.

@weiyangfb
Copy link
Contributor Author

weiyangfb commented Jun 8, 2018

Now the failure should appears:

00:21:26 ======================================================================
00:21:26 FAIL: test_hardshrink (test_torch.TestTorch)
00:21:26 ----------------------------------------------------------------------
00:21:26 Traceback (most recent call last):
00:21:26   File "/var/lib/jenkins/workspace/test/test_torch.py", line 5592, in test_hardshrink
00:21:26     self.assertEqual(torch.tensor([1, 0, 0, 0.6]).view(2, 2), data.hardshrink())
00:21:26   File "/var/lib/jenkins/workspace/test/common.py", line 307, in assertEqual
00:21:26     assertTensorsEqual(x, y)
00:21:26   File "/var/lib/jenkins/workspace/test/common.py", line 299, in assertTensorsEqual
00:21:26     self.assertLessEqual(max_err, prec, message)
00:21:26 AssertionError: tensor(0.5000) not less than or equal to 1e-05
00:21:26 
00:21:26 ----------------------------------------------------------------------
00:21:26 lambd = 0.3
00:21:26 lambd = 0.5
00:21:26 lambd = 0
00:21:26 Ran 508 tests in 29.749s
00:21:26 
00:21:26 FAILED (failures=1, skipped=123)
00:21:26 Traceback (most recent call last):
00:21:26   File "test/run_test.py", line 344, in <module>
00:21:26     main()
00:21:26   File "test/run_test.py", line 336, in main
00:21:26     raise RuntimeError(message)
00:21:26 RuntimeError: test_sparse failed!

The 3rd lambd equals 0, where it should be default value 0.5

@ezyang

@weiyangfb
Copy link
Contributor Author

For reasons I don't understand, this PR also allows nn.Hardshrink() to work in both of CPU and CUDA. For instance:

cuda = torch.device('cuda')
data = torch.zeros(2,2, device=cuda).fill_(0.3)
f = torch.nn.Hardshrink(0.3)
f(data)
-------------
tensor([[ 0.,  0.],
        [ 0.,  0.]], device='cuda:0')

Is nn.Hardshrink() borrowing the implementation from torch.hardshrink()? Did I miss something?

@ezyang
Copy link
Contributor

ezyang commented Jun 13, 2018

Is nn.Hardshrink() borrowing the implementation from torch.hardshrink()? Did I miss something?

That wouldn't surprise me! Have you checked the code?

@ngimel
Copy link
Collaborator

ngimel commented Jun 13, 2018

nn.Hardshrink is using def from functional.py, and you've told functional.py to call torch.hardshrink. All is good, I think.

@weiyangfb
Copy link
Contributor Author

weiyangfb commented Jun 14, 2018

@ezyang @ngimel You got it! Correcting what I said before, I think all looks good now :D

…tive_functions.yaml, and declare it at nn/functional.py
@weiyangfb
Copy link
Contributor Author

@pytorchbot retest this please

@weiyangfb
Copy link
Contributor Author

@pytorchbot retest this please

@weiyangfb
Copy link
Contributor Author

caffe CI test failures look not related, I will merge this PR

@soumith soumith merged commit ae55865 into pytorch:master Jun 14, 2018
@weiyangfb weiyangfb deleted the migrate_hardshrink_to_ATen branch June 22, 2018 18:12
}

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.

This comment was marked as off-topic.

@weiyangfb weiyangfb restored the migrate_hardshrink_to_ATen branch July 27, 2018 02:45
@weiyangfb weiyangfb deleted the migrate_hardshrink_to_ATen branch July 27, 2018 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement CUDA Hardshrink

5 participants