search

Reference-ScaLAPACK / scalapack

ScaLAPACK development repository
Other
127 stars 55 forks source link

Integer to real conversion in single precision scalapack gives wrong estimate of matrix size #99

Open sangallidavide opened 3 weeks ago

sangallidavide commented 3 weeks ago

AS the title says, when for example pcheevr is called in single precision, rwork(1) should contain the optimal size

*  RWORK    (local workspace/output) REAL  array,
*          dimension (LRWORK)
*          On return, RWORK(1) contains the optimal amount of
*          workspace required for efficient execution.

However the value is obtained from integer to real conversion https://github.com/Reference-ScaLAPACK/scalapack/blob/master/SRC/pcheevr.f#L592 which gives rounding error for integers bigger than 10^6

If the rounded value is subsequently used in input for phceevr (after conversion back to integer), this may give an error. See here the comment in a pull request of our code (and the following discussion): https://github.com/yambo-code/yambo/pull/117#issuecomment-2306887315

To fix this, it would be needed to change the above mentioned line (and other lines across the library) to a rounding procedure which always selects the closest bigger real number, e.g. rounding always done in excess.

sangallidavide commented 1 week ago

Any feedback on this? Is anyone still working on scalapack?

zerothi commented 1 week ago

Should ScaLAPACK force a newer LAPACK version, see e.g. https://github.com/Reference-LAPACK/lapack/pull/605

zerothi commented 1 week ago

Perhaps, until these things are settled, the methods should be duplicated in both codes (so scalapack can be linked with other lapack implementations).

sangallidavide commented 1 week ago

@zerothi in my case I'm compiling ScaLapack 2.2.1 by linking Lapack 3.12.0

I understand the fix is inside Lapack 3.12.0, but the issue appears anyway in phcheev. My understanding is that the call to https://github.com/Reference-LAPACK/lapack/blob/master/INSTALL/sroundup_lwork.f should be used in the scalapack source code as well.