Skip to content
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

Infinite loop in multiply when applied to NxN and N shaped arrays #267

Open
Caellian opened this issue Dec 11, 2024 · 2 comments
Open

Infinite loop in multiply when applied to NxN and N shaped arrays #267

Caellian opened this issue Dec 11, 2024 · 2 comments

Comments

@Caellian
Copy link

Caellian commented Dec 11, 2024

In browser environment, when I used multiply, it got stuck on NDIter in an infinite loop.

  • The cause is that NDArray arity isn't checked by the multiply function and it assumes it will work with two matrices. If one tries to multiple a matrix ([[...],[...],[...]]) with a vector [..], the c2 value is undefined.
  • This causes a subsequently constructed result matrix to have 0 size, i.e. undefined length.
  • Which isn't checked either when NDIter is constructed, resulting in an infinite loop because index > undefined is true (because undefined is cast to 0).

Solution

  • Check if vector is provided as second argument and turn it into a matrix.
    • Handle vectors from NDIter?
@Caellian Caellian changed the title Infinite loop on multiply in browser. Infinite loop in multiply when applied to NxN and N shaped arrays Dec 11, 2024
@mateogianolio
Copy link
Owner

mateogianolio commented Dec 13, 2024

Hey, thanks for creating an issue! Multiply is not as versatile as numpy's dot in python. As described in the docs, it "Multiplies two matrices x and y of matching dimensions. Accelerated with BLAS ?gemm.".

If you want to multiply a matrix with a vector, you need to first make the vector a column/row matrix and then perform the multiplication.

I think the more correct way to address this shortcoming is to make dot behave like numpy's dot.

EDIT: And also make multiply handle the infinite loop. It should instead throw an error.

@Caellian
Copy link
Author

Thanks for clarification. I ended up using gl-matrix because it was more in line with what I need algrebra for (WebGL), but it's much less ergonomic because it operates on TypedArrays directly.

I believe it would be good to note this in docs as well and point to dot function. I assume a.size.length !== b.size.length is enough to check for this case, but yeah, whatever doesn't make the whole browser freeze is great haha.

I'd like to point out that ability to distinguish between dot and multiply probably depends on previously used libraries. In my case, I'm coming from nalgebra which overrides the multiply operator so I'm probably expecting dot when I see multiply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants