RE: [eigen] Intel (R) MKL IE SpBLAS support in Eigen

["Zhukova, Maria" <maria.zhukova@xxxxxxxxx>]

Hi Cristoph,

Thanks for your comments!
Regarding sparse handle: the idea of IE SpBLAS handle is that it should be created just once and contain internal data related to the matrix structure, different optimizations, etc., which is used in subsequent calls, so it will be very inefficient to do create and destroy on-the-fly. 
That’s why I suggested adding this handle as an additional member and include creating routine in a place such as makeCompressed() function (because it is anyway required in order to make matrix format compatible with IE SpBLAS functionality). 
I will take care of all the remarks in a next patch version once I gather more feedbacks. 

Best regards,
Maria

-----Original Message-----
From: Christoph Hertzberg [mailto:chtz@xxxxxxxxxxxxxxxxxxxxxxxx] 
Sent: Wednesday, April 4, 2018 2:44 AM
To: eigen@xxxxxxxxxxxxxxxxxxx
Subject: Re: [eigen] Intel (R) MKL IE SpBLAS support in Eigen

On 2018-04-04 02:58, Edward Lam wrote:
> Hi Maria,
> 
> I think your new proposal sounds much better. I haven't looked at your 
> patch but please make sure the resource management is done properly 
> when the matrix is copied as well (ie. it's copy constructor and/or 
> operator=() method). [...].

In other words, obey the rule of 5/3/0:
http://en.cppreference.com/w/cpp/language/rule_of_three

I.e., ideally have a helper struct which obeys the rule of 5 and don't use custom copy/assignment implementations in the main class -- I'm aware that Eigen does not strictly follow that everywhere, e.g., SparseMatrix only obeys the rule of 3 at the moment.


Christoph



> 
> Thanks in advance for your hard work!
> 
> -Edward
> 
> On Tue, Apr 3, 2018, 8:46 PM Zhukova, Maria, <maria.zhukova@xxxxxxxxx>
> wrote:
> 
>> Hello Edward and thanks much for your feedback!
>>
>> This is a good point.
>> Actually, the correct place for creating IE SpBLAS handle is one of 
>> the key moments which I want to discuss.
>> The thing is -- all Intel (R) MKL IE SpBLAS functionality requires 
>> matrix to be stored in compressed row/column storage format. 
>> Necessary storage format can be obtained by calling SparseMatrix::makeCompressed() function.
>> So, I wonder if the better decision will be to create sparse handle 
>> inside the makeCompressed() and destroy in SparseMatrix destructor. 
>> What do you think about this option?
>>
>> Best regards,
>> Maria
>>
>> -----Original Message-----
>> From: Edward Lam [mailto:edward@xxxxxxxxxx]
>> Sent: Tuesday, April 3, 2018 4:28 PM
>> To: eigen@xxxxxxxxxxxxxxxxxxx
>> Subject: Re: [eigen] Intel (R) MKL IE SpBLAS support in Eigen
>>
>> Hi Maria,
>>
>> I'm only an Eigen user but I'm extremely happy at this effort to have 
>> more MKL wrapping in Eigen! As our application depends on Intel TBB, 
>> the only real way to use Eigen with multithreaded support is via MKL.
>>
>>   > A.createSparseHandle(); /* *NEW*: is used to create handle 
>> required for all IE  > SpBLAS routines */  > ...
>>   > A.destroySparseHandle(); /* *NEW*: is used to delete created 
>> handle */
>>
>> I think that this is very un-C++ like. I'm not sure why it was 
>> decided to be done this way. Why wouldn't the sparse handle be 
>> allocated within A itself and then deallocated during A's 
>> constructor? Note also that in C++, we need to worry about what 
>> happens when the matrix is copied so this state management needs to 
>> be within the matrix itself or else the semantics of these create/destroy handles are very troublesome.
>>
>> Best Regards,
>> -Edward
>>
>> On 4/3/2018 5:39 PM, Zhukova, Maria wrote:
>>> Hello Eigen community,
>>>
>>> My name is Maria Zhukova and I’m a software development engineer at 
>>> Intel ® MKL Sparse team.
>>>
>>> My team is interested in contributing into Eigen, so I’ve 
>>> investigated our possibilities and so far this is what I have:
>>> Eigen support different operations for sparse matrices stored in CSR 
>>> and CSC format which can be implemented on a basis of IE SpBLAS 
>>> kernels (please, refer to 
>>> https://software.intel.com/en-us/mkl-developer-reference-c-inspector
>>> -e
>>> xecutor-sparse-blas-routines
>>> for the general idea of interfaces)
>>> , basically we want to implement calls to our IE SpBLAS into next
>> operations:
>>   > ....
>>>
>>>                   SparseMatrix + SparseMatrix (mkl_sparse_?_add)
>>>                   SparseMatrix * DenseVector  (mkl_sparse_?_mv)
>>>
>>>                   SparseMatrix * DenseMatrix   (mkl_sparse_?_mm)
>>>
>>>                   SparseMatrix * SparseMatrix  (mkl_sparse_spmm), 
>>> and Triangular solve (mkl_sparse_?_trsv).
>>>
>>> I’ve already started with implementation of 
>>> sparse_time_dense_impl_mkl kernel which is based on mkl_sparse_?_mv (included in patch).
>>>
>>> This is how it will look like for user:
>>> *#include <Eigen/SpBLASSupport> *<-- *NEW:* IE SpBLAS include module
>>>
>>> void main () {
>>>     SparseMatrix<double, RowMajor> A;
>>>    Matrix<double, Dynamic, 1> x, y;
>>>
>>>     A.makeCompressed(); /* Convert matrix A into CSR/CSC format */
>>> *A.createSparseHandle();*/* *NEW*: is used to create handle required 
>>> for all IE SpBLAS routines */
>>>
>>> // support of IE SpBLAS is here
>>> y = beta*y + alpha*A*x; /* call to mkl_sparse_?_mv with operation = 
>>> SPARSE_OPERATION_NON_TRANSPOSE */ y = beta*y + 
>>> alpha*A.transpose()*x;
>>> /* call to mkl_sparse_?_mv with operation = 
>>> SPARSE_OPERATION_TRANSPOSE */ y = beta*y + alpha*A.adjoint()*x; /* 
>>> call to mkl_sparse_?_mv with operation = 
>>> SPARSE_OPERATION_CONJUGATE_TRANSPOSE */
>>>
>>> *A.destroySparseHandle();* /* *NEW*: is used to delete created 
>>> handle */ }
>>>
>>> I’ve attached a draft patch including all necessary changes and 
>>> would like to hear your feedback.
>>> Please, let me know if you have any questions and comments.
>>>
>>> Best regards,
>>> Maria
>>>
>>
>>
>>
>>
> 

--
  Dr.-Ing. Christoph Hertzberg

  Besuchsadresse der Nebengeschäftsstelle:
  DFKI GmbH
  Robotics Innovation Center
  Robert-Hooke-Straße 5
  28359 Bremen, Germany

  Postadresse der Hauptgeschäftsstelle Standort Bremen:
  DFKI GmbH
  Robotics Innovation Center
  Robert-Hooke-Straße 1
  28359 Bremen, Germany

  Tel.:     +49 421 178 45-4021
  Zentrale: +49 421 178 45-0
  E-Mail:   christoph.hertzberg@xxxxxxx

  Weitere Informationen: http://www.dfki.de/robotik
  -----------------------------------------------------------------------
  Deutsches Forschungszentrum fuer Kuenstliche Intelligenz GmbH
  Firmensitz: Trippstadter Straße 122, D-67663 Kaiserslautern
  Geschaeftsfuehrung: Prof. Dr. Dr. h.c. mult. Wolfgang Wahlster
  (Vorsitzender) Dr. Walter Olthoff
  Vorsitzender des Aufsichtsrats: Prof. Dr. h.c. Hans A. Aukes
  Amtsgericht Kaiserslautern, HRB 2313
  Sitz der Gesellschaft: Kaiserslautern (HRB 2313)
  USt-Id.Nr.:    DE 148646973
  Steuernummer:  19/672/50006
  -----------------------------------------------------------------------