Bug fixes and small improvements#211
Conversation
…sing internal data
…g number of threads
Codecov Report
@@ Coverage Diff @@
## master #211 +/- ##
=======================================
Coverage 89.66% 89.66%
=======================================
Files 57 57
Lines 3250 3250
=======================================
Hits 2914 2914
Misses 336 336Continue to review full report at Codecov.
|
|
|
||
| A_host.mmult(C_host, B_host); | ||
|
|
||
| // Convert matrix does not create a fill up all the members of a |
There was a problem hiding this comment.
| // Convert matrix does not create a fill up all the members of a | |
| // convert_matrix() does not initialize all the members of a |
and the copy/move assignment operator copies cusparse_handle and descr (or reinitializes them).
|
|
||
| // Downcast the operator | ||
| auto cuda_operator = | ||
| // Downcast the operator. We need to extract the diagonal and inverse it. |
There was a problem hiding this comment.
| // Downcast the operator. We need to extract the diagonal and inverse it. | |
| // Downcast the operator. We need to extract the diagonal and invert it. |
|
|
||
| // Change to COO format. The only thing that needs to be change to go from CSR | ||
| // to COO is to change row_ptr_dev with row_index_coo_dev. | ||
| // Change to COO format. The only thing that needs to be change to go from |
There was a problem hiding this comment.
| // Change to COO format. The only thing that needs to be change to go from | |
| // Change to COO format. The only thing that needs to be changed to go from |
| // of _matrix to the format supported by amgx | ||
| auto &amgx_matrix = const_cast<SparseMatrixDevice<value_type> &>( | ||
| *(cuda_operator->get_matrix())); | ||
| auto &amgx_matrix = *(cuda_operator->get_matrix()); |
There was a problem hiding this comment.
This is really confusing. In the end, matrix is changed heavily here. For me, it would make more sense if the constructor copies the object.
There was a problem hiding this comment.
I don't understand what's the problem here.
There was a problem hiding this comment.
Passing op as std::shared_ptr to a const object feels conceptually weird if we change the wrapped object.
There was a problem hiding this comment.
I see but
- I will not copy the object. This is the most expensive part memory-wise of the algorithm.
- We do have a few variables (include the data in
SparseMatrixDevice) that aremutableand while we shouldn't usemutablethis is out of the scope of this PR.
There was a problem hiding this comment.
Let's document these problems and move on.
|
I have updated the PR |
4 participants
These are multiple bug fixes and small changes made in #204