Add abs for ByteTensor and CharTensor. #16893

hameerabbasi · 2019-02-08T11:31:36Z

fmassa

This looks good to me, thanks a lot!

I have a few minor comments, mostly cosmetic changes.

Also, could you add some tests in

Lines 7055 to 7079 in f032962

    
           def test_abs(self): 
        
               def _test_abs(tensors_dict): 
        
                   for category, tensors in tensors_dict.items(): 
        
                       for data in tensors: 
        
                           switch = torch.rand(data.size()).mul(2).floor().mul(2).add(-1).type(data.dtype) 
        
                           res = torch.mul(data, switch) 
        
                           self.assertTensorsSlowEqual(res.abs(), data, 1e-16) 
        
               max_val = 1000 
        
               _test_abs(self._make_tensors((3, 4), val_range=(0, max_val))) 
        
               _test_abs(self._make_tensors((3, 5, 7), val_range=(0, max_val))) 
        
               _test_abs(self._make_tensors((2, 2, 5, 8, 2, 3), val_range=(0, max_val))) 
        
               _test_abs(self._make_tensors((1000, ), val_range=(0, max_val))) 
        
               _test_abs(self._make_tensors((10, 10, 10), val_range=(0, max_val))) 
        
               # Checking that the right abs function is called for LongTensor 
        
               bignumber = 2 ^ 31 + 1 
        
               res = torch.LongTensor((-bignumber,)) 
        
               self.assertGreater(res.abs()[0], 0) 
        
               # One of 
        
               rec = torch.randn(2, 2, 3, 7, 6, 2).type(torch.float64).clamp(0, 1) 
        
               val1 = rec.select(-1, -1).data[0][0][0].sum() 
        
               val2 = rec.select(-1, -1).data.abs()[0][0][0].sum() 
        
               self.assertEqual(val1, val2, 1e-8, 'absolute value')

?
The cases for uint8 and int8 are not tested by _test_abs because _make_tensors don't take those into account

pytorch/test/test_torch.py

Lines 112 to 117 in f032962

    
           def _make_tensors(self, shape, val_range=(-100, 100), use_floating=True, use_integral=True): 
        
               float_types = [torch.double, 
        
                              torch.float] 
        
               int_types = [torch.int64, 
        
                            torch.int32, 
        
                            torch.int16]

I think you can just add a few special cases there, instead of modifying _make_tensors.

aten/src/ATen/Declarations.cwrap

aten/src/TH/generic/THTensorMath.h

hameerabbasi · 2019-02-09T14:07:02Z

That would've been my next question, where to add tests! Glad to see you already answered that.

facebook-github-bot

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Fixes #15089 Pull Request resolved: pytorch/pytorch#16893 Differential Revision: D14020115 Pulled By: ezyang fbshipit-source-id: 6f3be6ed28d2d37667159be45959d400bc473451

hameerabbasi added 3 commits February 8, 2019 15:25

Added byte.

59061ce

Try to add char.

d2c438a

Fix compilation error and remove ignores.

91ecc07

fmassa reviewed Feb 8, 2019

View reviewed changes

aten/src/ATen/Declarations.cwrap Outdated Show resolved Hide resolved

aten/src/TH/generic/THTensorMath.h Outdated Show resolved Hide resolved

hameerabbasi added 3 commits February 9, 2019 19:01

Make cosmetic changes as suggested by @fmassa

22d3e84

Add vector variants of abs.

85c53eb

Add tests.

8c9c448

ezyang approved these changes Feb 10, 2019

View reviewed changes

facebook-github-bot reviewed Feb 10, 2019

View reviewed changes

facebook-github-bot closed this in 73d7ecd Feb 11, 2019

ezyang added open source merged labels Jun 24, 2019

vadimkantorov mentioned this pull request Nov 21, 2022

Edge case: CPU bool abs is not supported #89393

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add abs for ByteTensor and CharTensor. #16893

Add abs for ByteTensor and CharTensor. #16893

Uh oh!

hameerabbasi commented Feb 8, 2019

Uh oh!

fmassa left a comment

Uh oh!

Uh oh!

Uh oh!

hameerabbasi commented Feb 9, 2019

Uh oh!

facebook-github-bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	def test_abs(self):
	def _test_abs(tensors_dict):
	for category, tensors in tensors_dict.items():
	for data in tensors:
	switch = torch.rand(data.size()).mul(2).floor().mul(2).add(-1).type(data.dtype)
	res = torch.mul(data, switch)
	self.assertTensorsSlowEqual(res.abs(), data, 1e-16)

	max_val = 1000
	_test_abs(self._make_tensors((3, 4), val_range=(0, max_val)))
	_test_abs(self._make_tensors((3, 5, 7), val_range=(0, max_val)))
	_test_abs(self._make_tensors((2, 2, 5, 8, 2, 3), val_range=(0, max_val)))
	_test_abs(self._make_tensors((1000, ), val_range=(0, max_val)))
	_test_abs(self._make_tensors((10, 10, 10), val_range=(0, max_val)))

	# Checking that the right abs function is called for LongTensor
	bignumber = 2 ^ 31 + 1
	res = torch.LongTensor((-bignumber,))
	self.assertGreater(res.abs()[0], 0)

	# One of
	rec = torch.randn(2, 2, 3, 7, 6, 2).type(torch.float64).clamp(0, 1)
	val1 = rec.select(-1, -1).data[0][0][0].sum()
	val2 = rec.select(-1, -1).data.abs()[0][0][0].sum()
	self.assertEqual(val1, val2, 1e-8, 'absolute value')

	def _make_tensors(self, shape, val_range=(-100, 100), use_floating=True, use_integral=True):
	float_types = [torch.double,
	torch.float]
	int_types = [torch.int64,
	torch.int32,
	torch.int16]

Add abs for ByteTensor and CharTensor. #16893

Add abs for ByteTensor and CharTensor. #16893

Uh oh!

Conversation

hameerabbasi commented Feb 8, 2019

Uh oh!

fmassa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hameerabbasi commented Feb 9, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants