Skip to content
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

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

Open
chewxy opened this issue Sep 17, 2017 · 3 comments

Comments

@chewxy
Copy link
Member

chewxy commented Sep 17, 2017

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: gorgonia/gorgonia#133

@chewxy
Copy link
Member Author

chewxy commented Sep 17, 2017

Relevant issue and discussion: numpy/numpy#9394

@chewxy
Copy link
Member Author

chewxy commented Sep 17, 2017

@kabaka0 any input on the behaviours of masked tensors?

@chewxy
Copy link
Member Author

chewxy commented Sep 17, 2017

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

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

No branches or pull requests

1 participant