Interface inconsistency in createSubMatrix[Virtual]

[Jan Blechta]

Now there is

mat.createSubMatrix(irows, icols, submat=submat)
submat.createSubMatrixVirtual(mat, irows, icols)

Considering that the methods got a new name just yesterday this is a good opportunity to straighten up the interface. Either

submat.createSubMatrix(mat, irows, icols)
submat.createSubMatrixVirtual(mat, irows, icols)

or

mat.createSubMatrix(irows, icols, submat=submat)
mat.createSubMatrixVirtual(irows, icols, submat=submat)

is possible. For consistency with createSubMatrices the second options is preferable. If you agree, I can provide a pull request.

[Jan Blechta]

Second option is also preferable because first option would cause old user code submat.createSubMatrix(mat, irows, icols) which used to do a shallow copy silently switch to a deep copy.

[Lisandro Dalcin]

Sorry, I'm not following your. I think mat.createSubMatrixVirtual(irows, icols, submat=submat) is not possible, the C API has not reuse flag for it (it would make little sense, the virtual sumatrix is a lightweight object). The intended usage of createSubMatrix() is the following:

# submat is a brand new matrix, uses MAT_INITIAL_MATRIX internally.
submat = mat.createSubMatrix(rows, cols) # deep copy

# copy values from mat to preallocated submat, uses MAT_REUSE_MATRIX internally.
mat.createSubMatrix(rows, cols, submat=submat) # copy values from

Could you elaborate a bit more your proposal, and provide some example call explaining the semantics and intention?

[Jan Blechta]

Oh, you're right. I did not realize that for MatCreateSubMatrixVirtual(A,isrow,iscol,*submat) submat!=NULL is not reused in any way. Anyway the interface could look like mat.createSubMatrixVirtual(irows, icols, submat=submat) and it would do the same as now, i.e., if submat is None: PetscCLEAR(submat.obj).

That depends whether you want to have createSubMatrixVirtual more consistent with createSubMatri[x|ces] (self = input matrix) or create (self = output matrix). So make a decision and close as INVALID eventually.

[Lisandro Dalcin]

The difference here is that createSubMatrixVirtual() is like createAIJ(), it is a factory method in charge of creating a new matrix with a specific type (i.e "submatrix"), while createSubMatri{x|ces}() work quite differently. Despite the names being similar after the recent rename, they are quite different beasts. Then, I don't see the inconsistency (although I agree that the name similarity is unfortunate).