Skip to content

lapack/gonum: use crossover point for blocked/unblocked code; clean up parameter checks

Sebastien Binet requested to merge lapack/ilaenv-3 into master

Created by: vladimir-ch

This got bigger than expected. The starting point was checking the list of functions from #83 (closed):

  • Dgebrd
  • Dorgql
  • Dsytrd
  • Dgeqrf
  • Dgerqf
  • Dorgqr
  • Dgehrd
  • Dorglq
  • Dgelqf

that call Ilanenv(3, ...) and sometime chose to ignore the returned value.

While at that I cleaned up the parameter checks. Doing this

  • made the checks stricter for workspace query calls and so broke some tests which I had to fix,
  • revealed some inconsistencies, especially regarding work[0] value upon return in case of non-workspace query call. Some functions return lwkopt while other compute a separate iws value. I changed the code to follow the reference but we can either ask the reference about this or just use lwkopt everywhere.

In general, I think that the following order of initial checks is the most reasonable:

  1. Check numerical parameters like uplo, trans, m, n, k, lda, lwork and the length of work.
  2. Quick return if possible with setting work[0] = 1.
  3. Compute optimal work length and return if query call
  4. Check slice lengths

I could be convinced to move leading dimension checks to 4, or also in the opposite direction to move slice length checks to 1 although it might not be a good idea to be so strict as exemplified by Dorgqr.

Unlike with the other functions, in case of Dorgqr I had to move the lda check after the workspace query return. The reason is Dgesvd. Dorgqr is queried at several places in Dgesvd for workspace size with varying arguments. It is called as Dorgqr(m, n, ..., a, lda, ..., -1) and also as Dorgqr(m, m, ..., a, lda, ..., -1). This is obviously impossible to pass. The reference gets around this because it's column-major and the number of rows is the same for each call. I think that it would be more correct to call it in the second case with u, ldu but then the test for Dlapll fails because it does not need U and passes nil, 0 for it.

Eventually we could delete checkMatrix also here in lapack.

The unified diff is big but the individual commits are focused.

Fixes #83 (closed)

Please take a look.

Merge request reports