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

numpy scalar * array-like == performance horror #3375

Closed
inducer opened this issue May 28, 2013 · 4 comments · Fixed by #3501
Closed

numpy scalar * array-like == performance horror #3375

inducer opened this issue May 28, 2013 · 4 comments · Fixed by #3501

Comments

@inducer
Copy link
Contributor

@inducer inducer commented May 28, 2013

When I run this:

import numpy as np

class MyThing(object):
    def __init__(self, shape):
        self.shape = shape

    def __len__(self):
        return self.shape[0]

    def __getitem__(self, i):
        if not isinstance(i, tuple):
            i = (i,)
        if len(i) > len(self.shape):
            raise IndexError("boo")

        return MyThing(self.shape[len(i):])

    def __rmul__(self, other):
        print "RMUL"
        return self

print np.float64(5)*MyThing((3,3))

I get this:

RMUL
RMUL
RMUL
RMUL
RMUL
RMUL
RMUL
RMUL
RMUL
[[<__main__.MyThing object at 0x2298b90>
  <__main__.MyThing object at 0x2298bd0>
  <__main__.MyThing object at 0x2298c10>]
 [<__main__.MyThing object at 0x2298c50>
  <__main__.MyThing object at 0x2298c90>
  <__main__.MyThing object at 0x2298cd0>]
 [<__main__.MyThing object at 0x2298d10>
  <__main__.MyThing object at 0x2298d50>
  <__main__.MyThing object at 0x2298d90>]]

Is there a way to tell numpy, "no, don't worry about it, just call __rmul__ on the whole thing, instead of picking it apart?"

In my specific case, MyThing is an array-like object that lives on a GPU, and while it's possible (and not necessarily incorrect) to pick the array apart in this way, it's unexpected and has really terrible performance.

(sorry about the many edits)

@seberg
Copy link
Member

@seberg seberg commented May 28, 2013

If you define an __array_priority__ probably it does. Other then that this is the completly correct behaviour since if something appears to be not array aware (and this has no array priority/array attribute to indicate something like that), it needs to convert to an array first. And doing the calculation elementwise for object arrays is the correct thing to do.

Your object is a python sequence it is not something that indicates it knows about arrays to numpy. Numpy can convert it, but it cannot do any assumptions about it. It should be sufficient to define __array_priority__. There may be improvements possible in how numpy does these things, but this is not a numpy issue as is.

@seberg seberg closed this May 28, 2013
@inducer
Copy link
Contributor Author

@inducer inducer commented May 28, 2013

__array_priority__ fixes this. Thanks!

@inducer
Copy link
Contributor Author

@inducer inducer commented Jun 30, 2013

Actually, I spoke too soon. This here:

import numpy as np

class MyThing(object):
    __array_priority__ = 1000

    def __init__(self, shape):
        self.shape = shape

    def __len__(self):
        return self.shape[0]

    def __getitem__(self, i):
        print "GETITEM", i
        if not isinstance(i, tuple):
            i = (i,)
        if len(i) > len(self.shape):
            raise IndexError("boo")

        return MyThing(self.shape[len(i):])

    def __rmul__(self, other):
        print "RMUL"
        return self

print np.float64(5)*MyThing((3,3))

prints

GETITEM 0
GETITEM 0
GETITEM 0
GETITEM 0
GETITEM 1
GETITEM 2
GETITEM 1
GETITEM 2
GETITEM 0
GETITEM 0
GETITEM 1
GETITEM 2
GETITEM 1
GETITEM 0
GETITEM 1
GETITEM 2
GETITEM 2
GETITEM 0
GETITEM 1
GETITEM 2
RMUL

so the np.float64*MyThing operation still leisurely iterates through the entire array before it decides, whoops, you know what, screw this, let's just call __rmul__. If the real-life (GPU-ly) equivalent of MyThing has a few million entries, that goes back to being a performance horror.

(Forgot to say: I'd like to request that this bug be reopened.)

@seberg
Copy link
Member

@seberg seberg commented Jun 30, 2013

True, probably the check for array priority needs to be moved further to the front somewhere, this is not good. I guess the thing gets converted to an array, and only then it notices that the actual object knew what to do in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants