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

Memory Acess Error with fm.Blocks and fm.Sparse #81

Closed JanKirchhofTU closed 3 years ago

JanKirchhofTU commented 4 years ago

Say I have two sparse matrices and I want to construct a block matrix from those:

import scipy.sparse
import fastmat as fm

a = fm.Sparse(
scipy.sparse.csr_matrix((3, 5))
)
b = fm.Sparse(
scipy.sparse.csr_matrix((3, 5))
)

# This works as expected
c = fm.Blocks([[a,b], [a,b]])
# This works as expected
d = fm.Blocks([[a,b]])
# This crashes with a Memory Access Error
e = fm.Blocks([a,b])

I am running

on a linux system.

ChristophWWagner commented 4 years ago

This one is a tough one as it is a good example for an "ungraceful fail" and should produce a more meaningful error message.

Blocks expects a double list. However, since you can index into fastmat matrices this is what probably happens at the second level. Duck typing ftw! The problem, unfortunately, lies in deciding what not do disallow in order to get this thing cleaner without impacting duck typing magic too much.

My suggestion would be to specifically disallow fastmat.Matrix as second-level container.

Other suggestions?

SebastianSemper commented 4 years ago

Would go down the same route. The supplied list should always contain another list, but then we should also check, if at the third level we finally have a matrix.

On Mon, 13 Jan 2020 at 15:02, Christoph Wagner notifications@github.com wrote:

This one is a tough one as it is a good example for an "ungraceful fail" and should produce a more meaningful error message.

Blocks expects a double list. However, since you can index into fastmat matrices this is what probably happens at the second level. Duck typing ftw! The problem, unfortunately, lies in deciding what not do disallow in order to get this thing cleaner without impacting duck typing magic too much.

My suggestion would be to specifically disallow fastmat.Matrix as second-level container.

Other suggestions?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/EMS-TU-Ilmenau/fastmat/issues/81?email_source=notifications&email_token=AAINSRX22Z5PNGOYG2A3I7DQ5RYAJA5CNFSM4KFFIDZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIYZ47Q#issuecomment-573677182, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAINSRURPN5NQNQMKDFBD4DQ5RYAJANCNFSM4KFFIDZA .

ChristophWWagner commented 3 years ago

In e24862c such an error message, warning about broken second iteration levels in the structure passed to Blocks.__init__(), was added. Hope that's enough to mitigate crashes from this failure vector in the future. Closing. Feel free to reopen if problems still persist.