tensor icon indicating copy to clipboard operation
tensor copied to clipboard

MaskedTensor has some fundamental arithmetic issues if we follow Numpy's conventions.

Open chewxy opened this issue 8 years ago • 3 comments

From @chewxy on July 9, 2017 22:30

func TestMaskedPlay(t *testing.T) {
	a := New(WithShape(2, 2), WithBacking([]float64{1, 2, 3, 4}), WithMask([]bool{true, false, false, false}))
	t.Logf("a \n%v", a)
	t.Logf("%v | %v", a.mask, a.l)

	b := New(WithShape(2, 2), WithBacking([]float64{1, 2, 3, 4}))
	t.Logf("b \n%v", b)

	incr := New(WithShape(2, 2), WithBacking([]float64{100, 100, 100, 100}))
	StdEng{}.Add(a, b, WithIncr(incr))
	t.Logf("\n%v ", incr)
}

Results in:

	masked_test.go:7: a 
		⎡--   2⎤
		⎣ 3   4⎦
	masked_test.go:8: [true false false false] | 4
	masked_test.go:11: b 
		⎡1  2⎤
		⎣3  4⎦
	masked_test.go:21: 
		⎡100  104⎤
		⎣106  108⎦

By and large this makes sense. However, current tests fail mainly because the original APIs were built to be mirrors of Numpy's API for masked arrays. In Numpy:

>>> import numpy as np
>>> import numpy.ma as ma
>>> y = ma.array([1, 2, 3], mask = [0, 1, 0])

>>> incr = np.array([100,100,100])
>>> x = np.array([2,4,6])
>>> incr += x + y
>>> incr
array([103, 104, 109])
>>> x
array([2, 4, 6])
>>> y
masked_array(data = [1 -- 3],
             mask = [False  True False],
       fill_value = 999999)

>>> incr = np.array([100,100,100])
>>> incr += x * y
>>> incr
array([102, 104, 118])

The last answer is pretty WTF-ing. It violates an arithmetic convention (PEDMAS)

@kabaka0 care to weigh in? What should the correct way be?

Copied from original issue: chewxy/gorgonia#133

chewxy avatar Sep 17 '17 21:09 chewxy

Relevant issue and discussion: https://github.com/numpy/numpy/issues/9394

chewxy avatar Sep 17 '17 21:09 chewxy

@kabaka0 any input on the behaviours of masked tensors?

chewxy avatar Sep 17 '17 21:09 chewxy

Additionally, maskedDense seems to not transpose correctly (I'll actually write proper tests to see if that's the case)

chewxy avatar Sep 17 '17 21:09 chewxy