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

Floor division when using floats. #148

Closed scmc72 closed 9 years ago

scmc72 commented 9 years ago

I recently submitted a pull request (#1762) to iris for preserving lazy data when performing basic mathematical operations with cubes.

This lead me to discovering what is perhaps an inconsistency with the way biggus performs division.

In Python 2 the command 1.0 / 2.0 = 0.5, and the command 1 / 2 = 0. When using integers, python performs integer or 'floor' division. (Note: in Python 3, 1 / 2 = 0.5)

From what I can tell, it seems that biggus does not mimic this behaviour. For example this code:

import biggus
import numpy as np

ones = np.ones(5, dtype=np.float32)
fives = 5 * ones

bigones = biggus.NumpyArrayAdapter(ones)
bigfives = biggus.NumpyArrayAdapter(fives)

div = bigones / bigfives

print div
print div.ndarray()

Produces:

<Array shape=(5) dtype=dtype('float32') size=20 B>

[ 0.  0.  0.  0.  0.]

Here, it looks like biggus is performing floor division even when using floats.

Adding from __future__ import division to this code fixes this problem as it means that the / is interpreted as a 'true' division, however this does not fix the problem as I think biggus interprets the underlying operator.div as a floor division regardless of the dtype.

Any thoughts @pelson @rhattersley? Am I missing something?

pelson commented 9 years ago

Looks like https://github.com/SciTools/biggus/blob/master/biggus/__init__.py#L636 needs to be a little more sophisticated and should check the dtypes.

scmc72 commented 9 years ago

We might not actually need to check the dtypes because numpy can do it for us I think. Something like this?

def __div__(self, other):
        try:
            return divide(self, other)
        except TypeError:
            return NotImplemented
...

def divide(a, b):
    return _Elementwise(a, b, np.divide, np.ma.divide)

np.divide checks dtypes and selects the appropriate type of division.

Let me know what you think!

pelson commented 9 years ago

:+1: - yes, completely.

rhattersley commented 9 years ago

:+1:

pelson commented 9 years ago

Closed by #149.