Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MatNullSpace -> nullspace #276

Merged
merged 3 commits into from
Jun 5, 2023
Merged

MatNullSpace -> nullspace #276

merged 3 commits into from
Jun 5, 2023

Conversation

cmd8
Copy link
Contributor

@cmd8 cmd8 commented Jun 5, 2023

No description provided.

@prj-
Copy link
Member

prj- commented Jun 5, 2023

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

@cmd8
Copy link
Contributor Author

cmd8 commented Jun 5, 2023

Do we need both nearnullspace and MatNearNullSpace?

@prj-
Copy link
Member

prj- commented Jun 5, 2023

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-
Copy link
Member

prj- commented Jun 5, 2023

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
Copy link
Contributor Author

cmd8 commented Jun 5, 2023

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-
Copy link
Member

prj- commented Jun 5, 2023

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

@prj-
Copy link
Member

prj- commented Jun 5, 2023

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!

@prj- prj- changed the title MatSetNearNullSpace --> MatSetNullSpace MatNullSpace -> nullspace Jun 5, 2023
@prj- prj- merged commit 0a77803 into FreeFem:develop Jun 5, 2023
@cmd8 cmd8 deleted the patch-2 branch June 5, 2023 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants