hypre icon indicating copy to clipboard operation
hypre copied to clipboard

Comment for hypre_CSRMatrix.owns_data is inconsistent with hypre_CSRMatrixDestroy behavior

Open lahwaacz opened this issue 2 years ago • 3 comments

Based on this comment

   HYPRE_Int             owns_data;       /* Does the CSRMatrix create/destroy `data', `i', `j'? */

it seems that the owns_data attribute of hypre_CSRMatrix should control the ownership of all data, i, and j pointers. However, the hypre_CSRMatrixDestroy function deallocates the i array unconditionally:

      hypre_TFree(hypre_CSRMatrixI(matrix),      memory_location);
      hypre_TFree(hypre_CSRMatrixRownnz(matrix), HYPRE_MEMORY_HOST);

      if ( hypre_CSRMatrixOwnsData(matrix) )
      {
         hypre_TFree(hypre_CSRMatrixData(matrix), memory_location);
         hypre_TFree(hypre_CSRMatrixJ(matrix),    memory_location);
         /* RL: TODO There might be cases BigJ cannot be freed FIXME
          * Not so clear how to do it */
         hypre_TFree(hypre_CSRMatrixBigJ(matrix), memory_location);
      }

Is there a specific reason for this or is it just an oversight? I'd like to have a completely non-owning hypre_CSRMatrix handle that is populated with data owned by an external library.

lahwaacz avatar Apr 15 '22 19:04 lahwaacz

Good question. I need to look into it more carefully. Stay tuned. Thanks!

liruipeng avatar Apr 15 '22 20:04 liruipeng

@ulrikeyang @rfalgout Any thoughts on this. Why do we deallocate I unconditionally? What is the "definition" of OwnsData (J and Data)?

liruipeng avatar May 24 '22 22:05 liruipeng

@ulrikeyang @rfalgout Any thoughts on this. Why do we deallocate I unconditionally? What is the "definition" of OwnsData (J and Data)?

I can only guess. If the coefficient structure of a matrix changes, then the sizes of j and data change, but not i. However, I don't see an issue including i as part of the data controlled by the owns_data attribute. @ulrikeyang, do you remember why this "owns data" feature was originally added?

rfalgout avatar May 24 '22 22:05 rfalgout