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

BUG: dgbmv gives "(len(x)>offx+(trans==0?m-1:n-1)*abs(incx)) failed for 7th argument x" #18647

Closed
lciti opened this issue Jun 8, 2023 · 1 comment · Fixed by #18652
Closed
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.linalg
Milestone

Comments

@lciti
Copy link
Contributor

lciti commented Jun 8, 2023

I get an error that the size of the x argument is incorrect when using dgbmv.
See the following MWE where I am creating a 5×7 matrix in BLAS general band format with zero subdiagonals and two superdiagonals. When the matrix is multiplied by a vector of length 7 it works fine. When dgbmv is asked to compute the product of the transpose of the matrix by a vector of length 5, it gives an error.

>>> import numpy as np
>>> from scipy.linalg.blas import dgbmv

>>> A = np.ones((3, 7))
>>> x1 = np.ones(7)
>>> x2 = np.ones(5)

>>> print(dgbmv(5, 7, 0, 2, 1., A, x1, trans=0))
[3. 3. 3. 3. 3.]
>>> print(dgbmv(5, 7, 0, 2, 1., A, x2, trans=1))
error                                     Traceback (most recent call last)
Cell In[97], line 10
      6 x2 = np.ones(5)
      9 print(dgbmv(5, 7, 0, 2, 1., A, x1, trans=0))
---> 10 print(dgbmv(5, 7, 0, 2, 1., A, x2, trans=1))

error: (len(x)>offx+(trans==0?m-1:n-1)*abs(incx)) failed for 7th argument x

If I pass an unnecessarily longer argument as x, it works fine and produces the correct answer:

>>> x1[-2:] = np.NaN
>>> print(dgbmv(5, 7, 0, 2, 1., A, x1, trans=1))
[1. 2. 3. 3. 3. 2. 1.]

Note that the two extra elements of x1 (compared to x2) are not used, otherwise the NaNs would propagate to the outputs.
It looks like the code doing the work is fine but there is a problem with the code checking the size of the arguments.

I am using numpy=1.23.5 and scipy=1.10.1.

@lciti lciti changed the title dgbmv gives "(len(x)>offx+(trans==0?m-1:n-1)*abs(incx)) failed for 7th argument x" BUG: dgbmv gives "(len(x)>offx+(trans==0?m-1:n-1)*abs(incx)) failed for 7th argument x" Jun 8, 2023
@dschmitz89 dschmitz89 added defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.linalg labels Jun 8, 2023
@lciti
Copy link
Contributor Author

lciti commented Jun 9, 2023

I think the error is at line 95 of fblas_l2.pyf.src. Basically the same check is used for y and x while one is pre-multiplied and the other one is added (hence their length must be different for rectangular a).

90  check(len(y)>offy+(trans==0?m-1:n-1)*abs(incy)) :: y
...
95  check(len(x)>offx+(trans==0?m-1:n-1)*abs(incx)) :: x)

Unless I am missing something, the fix is easy and boils down to replacing == with != (or swapping m-1 and n-1) in line 95.

lciti pushed a commit to lciti/scipy that referenced this issue Jun 9, 2023
Given an m×n banded matrix A, linalg.blas.?gbmv does
y <- α A x + β y     if trans==0
y <- α A^T x + β y   if trans==1 (and A^H if trans==2)
Therefore x must be of length n when trans==0 and m otherwise.
This commit fixes the current code, which does the opposite.
It also includes a regression test, which fails for the current code
and passes for the new one.
Closes scipy#18647.
@ilayn ilayn added this to the 1.12.0 milestone Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.linalg
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants