Skip to content

BUG: A*B promotes to A@B but explicit A.__mul__(B) does not for np.ndarray and np.matrix #25213

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

Closed
chedatomasz opened this issue Nov 21, 2023 · 2 comments
Labels

Comments

@chedatomasz
Copy link

chedatomasz commented Nov 21, 2023

Describe the issue:

The Python docs on dunder methods with reflected operands says:
These functions are only called if the left operand does not support the corresponding operation and the operands are of different types.
“Does not support” here means that the class has no such method, or the method returns NotImplemented. Do not set the method to None if you want to force fallback to the right operand’s reflected method—that will instead have the opposite effect of explicitly blocking such fallback.
For operands of the same type, it is assumed that if the non-reflected method – such as add() – fails then the overall operation is not supported, which is why the reflected method is not called.

This behaviour seems to be violated when performing an operation like np.ndarray * np.matrix. This code evaluates like np.matrix.__rmul__(np.ndarray).
Confusingly, np.ndarray.__mul__(np.matrix) exists and can be called explicitly, leading to a different result.
AFAIK dispatching to np.matrix rmul is expected, as both left- and right-handed multiplicaitons on np.matrix are evaluated as matrix multiplication. However, according to the standard, calling through mul should also follow this behaviour, or follow the "Does not support" path described above.

I believe this bug happens because a*b bypasses __getattribute("__mul__")__ due to special method lookup rules - x.__mul__ goes through class __getattribute__, type(x).__mul__ goes through metaclass __getattribute__, while x*foo goes directly to x's __mul__ implementation. Unfortunately I do not have enough experience with the numpy codebase to check if this is really the case here.

Side note:
np.ndarray *= np.matrix leads to behaviour similar to np.ndarray.__mul__(np.matrix), just with the result being an array - this is confusing, but described in the docs and does not violate the standard. Debugging this is how I found the main issue I'm reporting.

Yes, I know np.matrix use is discouraged.
This might be related to #5844

Reproduce the code example:

import numpy as np
nda = np.ones((3,3))
mat = np.matrix((np.ones((3,1))))
infix_op = A*mat
print(infix_op)
# [[3.]
#  [3.]
#  [3.]]
print(type(infix_op))
# <class 'numpy.matrix'>

dunder_instance = A.__mul__(mat)
print(dunder_instance)
# [[1. 1. 1.]
#  [1. 1. 1.]
#  [1. 1. 1.]]
print(type(dunder_instance))
# <class 'numpy.matrix'>

dunder_class = np.ndarray.__mul__(A, mat)
print(dunder_class)
# [[1. 1. 1.]
#  [1. 1. 1.]
#  [1. 1. 1.]]
print(type(dunder_class))
# <class 'numpy.matrix'>

Error message:

No response

Runtime information:

1.23.5
3.10.12 (main, Jun 11 2023, 05:26:28) [GCC 11.4.0]

Context for the issue:

No response

@chedatomasz chedatomasz changed the title BUG: <Please write a comprehensive title after the 'BUG: ' prefix> BUG: A.__mul__(B) returns valid result different than A*B for np.ndarray and np.matrix Nov 21, 2023
@chedatomasz chedatomasz changed the title BUG: A.__mul__(B) returns valid result different than A*B for np.ndarray and np.matrix BUG: A*B promotes to A@B but explicit A.__mul__(B) does not for np.ndarray and np.matrix Nov 21, 2023
@mhvk
Copy link
Contributor

mhvk commented Nov 21, 2023

Python behaviour is for a subclass to have precedence. Since matrix is a subclass of ndarray, ndarray * matrix will directly go to matrix.__rmul__(ndarray).

@seberg
Copy link
Member

seberg commented Nov 21, 2023

Indeed, closing, the way numpy works is a bit layered, so can be surprising. But this is normal operator precedence behavior.
If anyone has a good argument why we cannot rely on Python preferring subclasses, we can reopen.

@seberg seberg closed this as completed Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants