EMS-TU-Ilmenau / fastmat

A library to build up lazily evaluated expressions of linear transforms for efficient scientific computing.
https://fastmat.readthedocs.io
Apache License 2.0
24 stars 8 forks source link

Error in type propagation of Product with chain scalar-matrix products #29

Closed JanKirchhofTU closed 6 years ago

JanKirchhofTU commented 6 years ago

When chaining multiple matrix-scalar products (i.e. when using the mul operator override on multiple matrices) the scalar datatype might not get propagated properly such that the resulting matrix datatype is insufficient to hold the multiplication products.

This can currently be avoided by consolidating matrices and scalars first using parantheses. However, the problem lies in THAT line https://github.com/EMS-TU-Ilmenau/fastmat/blob/66d9055cc83d89bfb9e610143cf132c095ed8478/fastmat/Product.pyx#L84 in the context when a fastmat matrix class gets multiplied with a fastmat product containing a scalar factor. Then the scalar value of the product gets correctly represented but the type is not propagated. Therefore the resulting type does not represent the necessary scalar type promotion.

Example: Consider a product of a complex-valued scalar S and two real-valued matrices A and B. S A B results in a Product of real dtype whereas S (A B) results in a Product of complex dtype.

Proposed solution: move the line https://github.com/EMS-TU-Ilmenau/fastmat/blob/66d9055cc83d89bfb9e610143cf132c095ed8478/fastmat/Product.pyx#L90 outside the if-clause to make sure it gets executed for Product classes as well (what it does not as it is right now).

[for further discussion: written by @ChristophWWagner]

ChristophWWagner commented 6 years ago

@JanKirchhofTU please check if the recent fix 5479044 fixes the issue by running the MWE of the original problem, which we set aside earlier, against a local checkout of the cython-fix branch with the said commit. If the problem does not occur anymore, please close the issue.

JanKirchhofTU commented 6 years ago

Solved with fix 547904474329fe924f63e8d261aa90d9b690d2ad.