-
Notifications
You must be signed in to change notification settings - Fork 166
Fix FacetSplit.update #4238
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
base: master
Are you sure you want to change the base?
Fix FacetSplit.update #4238
Conversation
1115dc3
to
b0fb4df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks generally good to me, just a couple of minor comments.
@@ -143,11 +144,10 @@ def _restrict_nullspace(nsp): | |||
|
|||
def update(self, pc): | |||
if hasattr(self, "_permute_op"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can we add a member for what configuration is being used instead of implicitly checking this based on what attributes we have? It would be clearer to the reader and more robust.
@@ -88,10 +88,11 @@ def initialize(self, pc): | |||
self.set_nullspaces(pc) | |||
self.work_vecs = self.mixed_opmat.createVecs() | |||
elif self.subset: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configuration member requested below would probably be set before this if-else block and then used for this block, and in the update method below.
Description
This PR fixes
FacetSplitPC
to correctly update nested preconditioners. In particular, nested python preconditioners were not correctly updated.