SciTools / biggus

:no_entry: [DEPRECATED] Virtual large arrays and lazy evaluation.
http://biggus.readthedocs.io/
GNU Lesser General Public License v3.0
54 stars 27 forks source link

Changes of array dtype under simple multiplication #163

Closed matthew-mizielinski closed 8 years ago

matthew-mizielinski commented 9 years ago

Added a failing test showing the change in dtype when multiplying an array by a simple factor. This issue was found by comparing dtypes of lazy loaded cubes in iris (after simple multiplication) with those for which the data had been loaded, showing an inconsistency.

This issue can be worked around by replacing calls such as array * 1 with array * np.float32(1) or cube * 1 by cube * np.float32(1) (assuming the original data type was float32), but is a little cumbersome.

pelson commented 9 years ago

Added a failing test showing the change in dtype when multiplying an array by a simple factor.

I'm actually a little surprised by the behaviour, but you are right:

(np.arange(4, dtype=np.float32) / 1.5).dtype dtype('float32')

The problem can be seen at in Elementwise, specifically:

            dtype = numpy_op(np.ones(1, dtype=array1.dtype),
                             np.ones(1, dtype=array2.dtype)).dtype

For a concrete case, this is implementing logic like:

>>> np.multiply(np.ones(1, dtype=np.float32), np.array([1.2])).dtype
dtype('float64')

But actually you want

>>> np.multiply(np.ones(1, dtype=np.float32), 1.2).dtype
dtype('float32')

I suspect we will find we don't need additional tests in this case - it is just the existing tests would need changing.

marqh commented 8 years ago

Hello @matthew-mizielinski

The issue you describe is to do with element wise operations (in this case multiply) working with biggus arrays and python numbers However, all the tests are explicitly passing in numpy objects, e.g. np.int32

I expect tests in the test file you have submitted to obey the numpy casting rules. So, Test.test_float32 passes Test.test_int32 fails.

I don't think this is a bug

A test set which passed in python numbers, 1, 2.2, should behave as numpy would behave. I think that #169 delivers this behaviour, and that this was the initial concern you raised

Please may you re-examine these tests, rebasing to master, and consider whether the tests are addressing the behaviour you want them to.

I would like to have my assertion that #169 fixes this problem confirmed or challenged

many thanks marq

pelson commented 8 years ago

I think this issue was solved by #169. Please feel free to re-open if I'm wrong.