PythonOptimizers / cysparse

Python/Cython library to replace PySparse
http://PythonOptimizers.github.io/cysparse
7 stars 3 forks source link

__rmul__ not triggered #237

Closed ghost closed 8 years ago

ghost commented 8 years ago

Method defined inside a matrix class:

def __add__(self, B):
        """
        Return a :class:`SumProxy`.

        Returns:
            A :class:`SumProxy`, i.e. a proxy to a matrix-like sum.

        """
        print "type of B: %s" % type(B)
        print "type of self: %s" % type(self)
        return SumProxy(self, B)

Code tested: 2 + A

This should normally trigger a NotImplementedError and Python should look at the __radd__ method of A. What happens instead is that the __mul__ method of A is called and this is what is printed:

type of B: <type 'cysparse.sparse.csc_mat_matrices.csc_mat_INT64_t_FLOAT64_t.CSCSparseMatrix_INT64_t_FLOAT64_t'>
type of self: <type 'float'>

Any clues?

@dpo @syarra

dpo commented 8 years ago

That seems very wrong...

dpo commented 8 years ago

+ should never call __mul__ (I think there's some confusion between mul and add in your message). Example:

class Truc(object):

    def __add__(self, other):
        print 'this is Truc.__add__'
        return Truc()

    def __radd__(self, other):
        print 'this is Truc.__radd__'
        return Truc()
In [1]: from truc import Truc

In [2]: t = Truc()

In [3]: t + 2
this is Truc.__add__
Out[3]: <truc.Truc at 0x10cadc310>

In [4]: 2 + t
this is Truc.__radd__
Out[4]: <truc.Truc at 0x10cadc650>

That's just how the MRO works.

ghost commented 8 years ago

Yes, I was talking about __add__. What you wrote is how it is supposed to work but it doesn't with the Cython code. Didn't dig further into the generated C code.

ghost commented 8 years ago

Interesting: on your small example compiled with Cython, this is what I get:

>>> a + 2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'A.A' and 'int'
>>> 2 + a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'A.A'

A.A is your Truc type.

ghost commented 8 years ago

Got it:

Arithmetic methods Arithmetic operator methods, such as add(), behave differently from their Python counterparts. There are no separate “reversed” versions of these methods (radd(), etc.) Instead, if the first operand cannot perform the operation, the same method of the second operand is called, with the operands in the same order.

This means that you can’t rely on the first parameter of these methods being “self” or being the right type, and you should test the types of both operands before deciding what to do. If you can’t handle the combination of types you’ve been given, you should return NotImplemented.

This also applies to the in-place arithmetic method ipow(). It doesn’t apply to any of the other in-place methods (iadd(), etc.) which always take self as the first

argument.

ghost commented 8 years ago

It's here: http://docs.cython.org/src/userguide/special_methods.html#arithmetic-methods

dpo commented 8 years ago

That's confusing.

ghost commented 8 years ago

Yes, it is but now we know. We are now able to use expressions like this one:

(3 * A - 2 * A * 1) * A.T

This is possible with all matrix element types, LL, CSC and CSR, and A.T, A.conj, A.adj. You can mix everything together. It needs some testing though (soon, probably next week).

Notice how you can multiply a matrix at both ends: 2 * A and A * 2. You can even combine both: 2 * A * 2 which doesn't create a new operator but updates the current one, i.e. 2 * A * 2 is just the same as 4 * A.

ghost commented 8 years ago

Oops, A.adj is in fact A.H.