TRIQS / triqs

a Toolbox for Research on Interacting Quantum Systems
https://triqs.github.io
GNU General Public License v3.0
138 stars 72 forks source link

pytriqs '*' operator is dot product -- not pythonic #599

Open DerWeh opened 6 years ago

DerWeh commented 6 years ago

Correct me if I am wrong, but it seems to me that you overloaded * with matrix multiplication. This is very contra-intuitive from a python point of view. I think it should be depreciated and a dot function should be introduced.

The reason is simply consistency. In numpy, which is the basis for most scientific calculations in python, * is element-wise multiplication.

PEP 465 even introduces a dedicated infix operator for matrix multiplication (which is part of Python 3.5's syntax). This is the @ operator.

Hence, I think using * for matrix multiplication reduces readability. People familiar with numpy or python would expect and element-wise product.

HugoStrand commented 6 years ago

Dear DerWeh,

In principle I agree with you. However, if you take a rank 2 numpy.ndarray and you turn it into a matrix,

import numpy as np
A = np.random.random((5, 5))
B = A * A # element wise product
M = np.mat(A)
C = M * M # matrix product

I think the rationale for a matrix valued greens function is that it is matrix valued, and hence default to matrix multiplication.

Do you have a more explicit example on how this gives unexpected behavior?

Best, Hugo

parcollet commented 5 years ago

Yes, Gf are matrix valued by default. Probably we should use np.matrix instead of np.array ? It would make the API more consistent. Would it break code ? @HugoStrand : Can you say if this replacement would be affect codes ? @Wentzell : opinion ?

Wentzell commented 5 years ago

Probably we should use np.matrix instead of np.array ? It would make the API more consistent.

I agree that this would be much more consistent! We should explore if and how much code this would break..

Wentzell commented 4 years ago

Unfortunately the use numpy matrix is deprecated. This is unfortunate because numpy matrix would be the proper solution here.