DenseMatrix is incompatible with LAPACK calls

Issue #338 resolved
Nils Deppe created an issue

Hi Klaus,

At some point DenseMatrix started adding padding without giving any user control over whether or not that is done. This makes Blaze incompatible with LAPACK calls, which is a big pain for us (and presumably other people). Would it be reasonable to fix this? We have found an workaround where we change to column major, and then change all the LAPACK calls in the entire code base. Nevertheless, being able to use Blaze more easily with LAPACK would be nice.

Also, since the padding was never specified as part of the API, I guess it’s arguable whether or not this counts as an API change, but it certainly is in a practical sense. Would it maybe be possible to leave such breaking changes to major version bumps?

Thanks!

Best,

Nils

Comments (9)

  1. Klaus Iglberger

    Hi Nils!

    Thanks for creating this issue. I agree that silently adding padding would be a breaking API change and would be a change reserved for major version bumps. However, there hasn’t been a breaking API change with regards to padding, though, and from my point of view there is no incompatibility with LAPACK. Allow me to explain:

    • DenseMatrix is a CRTP base class for all kinds of dense matrix (see the wiki), i.e. DenseMatrix does not have anything to do with padding. Do you mean DynamicMatrix?
    • All dense matrix classes (StaticMatrix, HybridMatrix, and DynamicMatrix), use padding since version 1.0. Padding is the reason why the interface of dense matrices doesn’t provide a begin() and end() function for the traversal of the entire matrix (see our discussion in issue #178).
    • The API of all dense matrices provides the necessary functions to deal with padding: the rows() function returns the current number of rows, the columns() function returns the current number of columns and the spacing() function returns the total number of elements of a row or column (depending on the storage order), including padding elements. The following example demonstrates this (assuming AVX):
    blaze::DynamicMatrix<double,blaze::rowMajor> A( 2UL, 3UL );
    rows(A);     // Returns 2
    columns(A);  // Returns 3
    spacing(A);  // Returns 4 (which is the number of elements in an AVX SIMD pack and thus the minimum number of elements in a row with padding)
    
    • Blaze itself builds on LAPACK and uses LAPACK calls for several dense matrix operations (decomposition, inversion, eigenvalues, singular values, …). Thus the compatibility to LAPACK is and always has been essential for Blaze. It also provides a large number of LAPACK wrappers (see the wiki), some of which work directly on any kind of dense matrix (see for instance the LU decomposition functions and inversion functions):
    blaze::DynamicMatrix<double,blaze::rowMajor> A( 101UL, 101UL );  // A is internally represented as 101x104 matrix (i.e. three padding elements)
    // ... Initialization of A
    std::unique_ptr<int[]> ipiv( new int[100] );
    getrf( A, ipiv.get() );  // LU decomposition of A, using 104 as argument to 'lda'
    getri( A, ipiv.get() );  // Inversion of A, using 104 as argument to 'lda'
    
    • The only change with regard to padding that we recently introduced in Blaze 3.7 was the ability for StaticMatrix to disable (!) padding (see the wiki). The interface was merely extended, though, and no defaults were changed.

    I hope the explanation helps to resolve a possible misunderstanding. If not, could you please provide me with a minimum example where and how Blaze is not compatible to LAPACK? That would help me to get a better understanding of the real issue. Thanks again,

    Best regards,

    Klaus!

  2. Jordan Moxon

    Hi Klaus,

    Thanks for your reply. Nils was describing some trouble that we had when we recently moved to supporting Blaze 3.7 in our codebase.

    You’re correct that we are referring specifically to DynamicMatrix, which to the best of our understanding does not support adjusting the padding policy via a template argument. I think it is true that in principal the API specification made no guarantee for the padding policy for earlier versions, but our experience appears to indicate that the actual data layout changed nontrivially between 3.6 and 3.7, at least on certain machines we tested on.

    Previously, we had a custom matrix class that inherits from the Blaze DynamicMatrix as:

    template <typename T, bool SO = blaze::defaultStorageOrder>
    class DenseMatrix : public blaze::DynamicMatrix<T, SO>
    

    And our old LAPACK calls were of the flavor:

      dgesv_(&rhs_vector_size, &number_of_rhs, matrix_operator->data(),
             &output_vector_size, pivots->data(), rhs_in_solution_out->data(),
             &output_vector_size, &info);
    

    where the matrix_operator is the derived class of DynamicMatrix - note that previously using the LDA argument of LAPACK as simply the extent of the matrix was sufficient. This particular use-case no longer functioned when including Blaze 3.7 instead.

    The alteration to this set to get back to a working linear algebra system was to first move to

    template <typename T, bool SO = blaze::columnMajor>
    class DenseMatrix : public blaze::DynamicMatrix<T, SO>
    

    and to adjust the LAPACK call to privide matrix_operator->spacing() to the LDA parameter (and similar changes for several other calls). It was my understanding that the layout assumptions of LAPACK calls would not have sufficient stride options to support the row-major padding from blaze:defaultStorageOrder - Is that mistaken?

    Thanks for your help in getting to the bottom of this!

    Cheers

    Jordan

  3. Klaus Iglberger

    Hi Jordan!

    Thanks for the additional explanation. By now I have a suspicion what could cause a problem: Could you please check if you perhaps on the BLAZE_USE_PADDING compilation switch?

    Best regards,

    Klaus!

  4. Jordan Moxon

    Hi Klaus,

    I’ve tried the example in question again with the compiler switch

    #define BLAZE_USE_PADDING 0
    

    set in our configuration header, and confirmed with tracing output that it is indeed set to zero, but the derived class ofDynamicMatrix still exhibits padding when we access the memory layout directly via the data member function. The size of the matrix columns (still imposing blaze::columnMajor in this test) appears to be padded out to an integer multiple of 4 on my machine.

    Thanks again!

    Jordan

  5. Klaus Iglberger

    Hi Jordan!

    Thanks, that confirms my suspicion. Unfortunately the BLAZE_USE_PADDING flag is no longer used and was supposed to be replaced by an instance specific configuration. However, I didn’t realize that now the state of affairs is caught between issue #134, which made it into Blaze 3.7 and issue #307, which didn’t quite make it. In summary, the error is on my side and I will reactivate the flag within the next 24 hours. I apologize for the inconvenience and your lost time!

    Still, I will mark the BLAZE_USE_PADDING flag as deprecated, since the decision to have a global padding configuration was not a good one to begin with. I will keep the flag for the configuration of DynamicVector and DynamicMatrix until at least Blaze 3.8, but afterwards remove it. By then you’ll have to update your LAPACK calls or to use the new instance-specific configuration options.

    Thanks a lot for pointing out this defect,

    Best regards,

    Klaus!

  6. Klaus Iglberger

    Commits 84ee25f and 020565f reintroduce the BLAZE_USE_PADDING compilation flag for the DynamicVector and DynamicMatrix class templates. The fix is immediately available via cloning the Blaze repository and will be officially released in Blaze 3.8.

  7. Log in to comment