Magma change storage_type error in GPU

[mikemike10212003]

Dear all,
I use magma_zmconvert function in GPU.
Try dense to CSR and CSR to DENSE in GPU or CPU.
In CPU,
DENSE to CSR of CPU is based on RowMajor.
CSR to DENSE of CPU is based on RowMajor.
CSR to DENSE of GPU is based on ColMajor.
I don't know how DENSE to CSR of GPU does, because it will return just dimension or some element not in origin array.
For example,
A= [1, 2, 3; 4, 5, 6];
array of A in CPU = [1 2 3 4 5 6]
Convert it to CSR in CPU, transfer it to GPU and then convert it to DENSE.
array will become [1 4 2 5 3 6]
So the matrix will become [ 1 4 2; 5 3 6]
I think it needs to change function of CPU to colmajor or function of GPU to rowmajor.
I guess changing function of CPU to colmajor is better because other routine are based on colmajor.
But I need to check whether it will impact another function. After checking it, I will try to fix it.
Thanks.

The following is my code.

Code: Select all

#include <iostream>
#include <string>
#include <complex>
#include <cstdio>
#include <cstdlib>
#include <sstream>
#include "cublas_v2.h"
#include "magma_v2.h"
#include "magmasparse.h"
#include "magma_lapack.h"

int main(){
   magma_init();
   magma_int_t dev=0;
   magma_queue_t queue=NULL;
   magma_queue_create(dev, &queue);
   magma_z_matrix oA, A;
   magma_z_matrix dA;
   int m=5, n=3;
   magmaDoubleComplex *val;
   val=new magmaDoubleComplex[m*n];
   magma_z_matrix cA,ddA, cdA, ccA,csrdA, csrA;
   for (int i=0; i<m*n; i++){
      val[i]=MAGMA_Z_MAKE(i,0);
   }
   magma_zvset(m,n,val,&oA,queue);
   magma_zmconvert(oA,&A,Magma_DENSE, Magma_CSR, queue);
   write_z_csrtomtx(A,"C.mtx",queue);
   magma_zmtransfer( A, &dA, Magma_CPU, Magma_DEV, queue );
   magma_zmtransfer( dA, &cA, Magma_DEV, Magma_CPU, queue );
   write_z_csrtomtx(cA,"CTT.mtx",queue);
   magma_zmconvert(dA, &ddA, Magma_CSR, Magma_DENSE, queue);
   magma_zmtransfer(ddA, &cdA, Magma_DEV, Magma_CPU, queue);
   for (int i=0;i<m*n;i++)
      std::cout<<MAGMA_Z_REAL(cdA.val[i])<<" ";
   magma_zmconvert(cdA, &ccA, Magma_DENSE, Magma_CSR, queue);
   write_z_csrtomtx(ccA,"CTCTC.mtx",queue);
   magma_zmconvert(ddA, &csrdA, Magma_DENSE, Magma_CSR, queue);
   magma_zmtransfer(csrdA, &csrA, Magma_DEV, Magma_CPU, queue);
   write_z_csrtomtx(csrA,"CSRA.mtx",queue);
   return 0;
}



[hartwig anzt]

Thank you for pointing this out!

The cuSPARSE routine that is called in the converter converts the CSR matrix to a dense matrix stored in col-major. Col-major is also the LAPACK standard for dense matrices, so this is correct.
Unfortunately, in MAGMA-sparse we always assume the dense matrices to have row-major storage. This is an inconsistency we will have to address some time in the future. The reason we use row-major for the dense matrices in sparse LA is that the only dense matrices we usually deal with are block-vectors - like A * x = y where x=[x_1 x_2 x_3 ...]. In this case, row-major storage gives much better performance for the matrix vector product.

For now I fixed it by using the CPU workaround: copy the matrix back to the CPU, convert it to dense, copy it back to the GPU. Not very elegant, but it works.

Thanks again, I will keep you posted if we change this at some point in future.

Hartwig

[mikemike10212003]

Dear all,
magma-2.2.0 moves data to CPU, converts it and moves data to GPU based on Rowmajor.
Thus, I guess I can change the part safety and it will not let another function get error.
I use similar function in magma-2.1.0 and fix it with correct parameters.
The main idea is using transpose but with another size.
magmablas_ztranspose handles transpose. It should be a GPU function, right?
The attachment can convert CSR to DENSE and DENSE to CSR just in GPU.
I have tested small case and they are correct.
Note. I use additional magmaDoubleComplex_ptr dT in if-else to as a temp storage.
I am not sure whether I should put it in head or in if-else.

Mike

[hartwig anzt]

Mike,
this looks good! A few comments, also as answer to your questions.

The error handling works the following way: If any of the function returns an error, the "CHECK" will catch this, and proceed at the "cleanup" at the end of the routine. The intention is is free all memory allocated, and return the error upwards. If you now free your temporary storage dT only inside the if-clause, and a routine before freeing the memory fails, it goes right to the cleanup, and dT never gets freed. Hence, you should have the magma_free( dT ) also in the cleanup phase. This answers your question: you should declare dT in the beginning. Also, beside declaring it, you should initialize it with "dT = NULL;" The reason is that in case you go through cleanup without having allocated dT before (imagine another branch or an error), it does not know what to free.

other than that, it looks very good! Thanks, Hartwig

[mikemike10212003]

Dear Hartwig,
Thanks for your reply.
I changed the dT position to follow the error handling.
Thanks,
Mike