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

svd not working for matrices with a single column #835

Closed
loiseaujc opened this issue Jun 14, 2024 · 6 comments · Fixed by #836
Closed

svd not working for matrices with a single column #835

loiseaujc opened this issue Jun 14, 2024 · 6 comments · Fixed by #836
Labels
bug Something isn't working

Comments

@loiseaujc
Copy link

Description

Consider the following MWE

program main
  use stdlib_kinds, only: dp
  use stdlib_linalg, only: svd
  implicit none

  integer, parameter :: m = 2, n = 1
  ! Sigular value decomposition of A.
  real(kind=dp) :: A(n, n)
  real(kind=dp) :: U(n, n), S(n), Vt(n, n)

  ! Random matrix.
  call random_number(A) 

  ! Call to stdlib.
  call svd(A, S, U, Vt)

end program main

Using gfortran 13.2.0, I get this run time error:

At line 75528 of file build/dependencies/stdlib/src/stdlib_linalg_lapack_d.F90
Fortran runtime error: Index '2' of dimension 1 of array 'a' above upper bound of 1

Error termination. Backtrace:
#0  0x5621220d2c4a in __stdlib_linalg_lapack_d_MOD_stdlib_dgesdd
	at build/dependencies/stdlib/src/stdlib_linalg_lapack_d.F90:75528
#1  0x56212190442b in __stdlib_linalg_MOD_stdlib_linalg_svd_d
	at build/dependencies/stdlib/src/stdlib_linalg_svd.f90:483
#2  0x5621218f77c1 in MAIN__
	at app/main.f90:15
#3  0x5621218f780c in main
	at app/main.f90:2
<ERROR> Execution for object " MWE_stdlib_lstsq " returned exit code  2
<ERROR> *cmd_run*:stopping due to failed executions

Expected Behaviour

Computing the SVD of a column vector is admittedly a contrived example but this piece of code is part of larger subroutine in LightKrylov iteratively computing the SVD of large-scale matrices using Lanczos Bidiagonalization. Note that if a row vector is considered instead of a column vector, the MWE runs perfectly.

Pinging @perazz, @jalvesz, @jvdp1.

Version of stdlib

0.6.1

Platform and Architecture

Linux with Ubuntu 22.04

Additional Information

No response

@loiseaujc loiseaujc added the bug Something isn't working label Jun 14, 2024
@jalvesz
Copy link
Contributor

jalvesz commented Jun 14, 2024

@loiseaujc I noticed that in your mwe you use only n to define the matrices. shouldn't m be the first dimension of every matrix? tried that here https://godbolt.org/z/vf6bxqezd and it works

@loiseaujc
Copy link
Author

Oups, my bad. I'll have to double check in LightKrylov then. I'll close this issue for the moment.

@loiseaujc
Copy link
Author

loiseaujc commented Jun 14, 2024

Actually, if m = 1, and n = 1, I still get an error.

program main
  use stdlib_kinds, only: dp
  use stdlib_linalg, only: svd
  implicit none

  integer, parameter :: m = 1, n = 1
  ! Sigular value decomposition of A.
  real(kind=dp) :: A(m, n)
  real(kind=dp) :: U(m, m), S(n), Vt(n, n)

  ! Random matrix.
  call random_number(A) 

  ! Call to stdlib.
  call svd(A, S, U, Vt)

end program main

@loiseaujc loiseaujc reopened this Jun 14, 2024
@perazz
Copy link
Contributor

perazz commented Jun 14, 2024

Sorry all for the late reply, I'm abroad on business travel the whole week. The error apparently comes from a LAPACK limitation, as a(2,1) is always accessed: in DGESDD,

                    ! produce r in a, zeroing out other entries
                    call stdlib_dlaset( 'L', n-1, n-1, zero, zero, a( 2, 1 ), lda )

but it should not take place as the requirement is LDA >= max(1,M). So, I will investigate what matrix option is being used for this edge case.

EDIT: Yes: that code segment is called part of this branch:

           if( m>=n ) then
              ! a has at least as many rows as columns. if a has sufficiently
              ! more rows than columns, first reduce using the qr
              ! decomposition (if sufficient workspace available)

so all provided matrix sizes are OK according to the inputs. stdlib_dlaset should return without doing anything because the input n-1 == 0, but of course the compiler's bound checking complains because we are referencing address a(2,1), which does not exist. So it indeed looks like you've found an issue with LAPACK: not technically a bug, but rather a F77-style edge case that modern compilers complain about. We must check if any of the other options (I.e. the ‘reduced matrices’ one) allows to continue bypassing this error

@loiseaujc
Copy link
Author

loiseaujc commented Jun 14, 2024

Oh I see. I was using gesvd in LightKrylov which doesn't seem to suffer from this issue. I'll switch to stdlib_svd nonetheless and simply add a if (k > 1) call svd(A, S, U, Vt) since in practice it is very unlikely that a Krylov method converges in a single itertion. No big deal. Closing this issue then.

@perazz
Copy link
Contributor

perazz commented Jun 15, 2024

I think we need to fix this @loiseaujc, svd must work for all cases, including a 1-sized matrix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants