FreeFem / FreeFem-sources

FreeFEM source code
https://freefem.org/
Other
746 stars 187 forks source link

MatNullSpace -> nullspace #276

Closed cmd8 closed 1 year ago

prj- commented 1 year ago

That's not what's needed, because in some scenario, you truly specify the MatNearNullSpace. We need another named parameter.

cmd8 commented 1 year ago

Do we need both nearnullspace and MatNearNullSpace?

prj- commented 1 year ago

My bad, I just realized there is already what's needed, I think. For near nullspace, there is the named parameter nearnullspace. For nullspace, there is the named parameter MatNullSpace. But in that case, we should call MatSetNullSpace(), not MatSetNearNullSpace().

prj- commented 1 year ago
diff --git a/plugin/mpi/PETSc-code.hpp b/plugin/mpi/PETSc-code.hpp
index a00631fe..0904f6bd 100644
--- a/plugin/mpi/PETSc-code.hpp
+++ b/plugin/mpi/PETSc-code.hpp
@@ -2540,3 +2540,4 @@ namespace PETSc {
             MatNullSpaceCreate(PetscObjectComm((PetscObject)ptA->_petsc), PETSC_FALSE, std::max(dim, dimPETSc), ns, &sp);
-            MatSetNearNullSpace(ptA->_petsc, sp);
+            if (dim) MatSetNearNullSpace(ptA->_petsc, sp);
+            else MatSetNullSpace(ptA->_petsc, sp);
             MatNullSpaceDestroy(&sp);
cmd8 commented 1 year ago

Yes, agreed. Nice fix. Do you think that parameter should be changed from nearnullspace to MatNearNullSpace for consistency? Or perhaps MatNullSpace should be renamed nullspace?

prj- commented 1 year ago

Probably the latter, since it's less frequently used, so there is a lower chance of breakage.

prj- commented 1 year ago

Could you please revert my initial change suggested in private and then apply the diff from above? Then, I think we'll be good to go!