Skip to content

Conversation

@sv2518
Copy link
Contributor

@sv2518 sv2518 commented Oct 25, 2021

This PR enables us to do specify some (local) preconditioners to the (local) operators in the local solves, used in the HybridizationPC.

All local preconditioners are controlled with PETSc options. The corresponding prefix is _lmi_ which stands for local mixed inverse. There are options to control which preconditioner to use and if it is replacing the original operator (preonly) or not. All of this is locally matrix-explicit only, the locally matrix-free PR will be coming in later.

The two options of local preconditioners are
a) a Jacobi preconditioner and
b) a user-supplied operator. Note that b) was originally in a separate PR, see #2240.

The first commit d8a3e15 is only refactoring and also renaming some existing options to look more PETSc like. Then there follow some infrastructural additions to Slate and its translations into lower levels, which I needed for the local diagonal preconditioners.

The last commit 10bdca7 is a big one. It includes adding more infrastructure into the SchurComplementBuilder class to deal with the new preconditioners, including the handling of possible options which are explained in detail in the SchurComplementBuilder class description. Testing is part of this commit too (I test both getting the right solutions and handling the options correctly in one go)

@sv2518 sv2518 force-pushed the sv/local-preconditioners branch 2 times, most recently from 60a3722 to 4ab62c6 Compare October 26, 2021 14:40
@sv2518 sv2518 marked this pull request as ready for review October 26, 2021 14:44
@sv2518
Copy link
Contributor Author

sv2518 commented Oct 26, 2021

Okay this should be ready for review now.

Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks nice, and had a bunch of comments that might be helpful

@sv2518 sv2518 force-pushed the sv/local-preconditioners branch from 4c9cc85 to 99a963d Compare November 1, 2021 10:46
@sv2518 sv2518 force-pushed the sv/local-preconditioners branch 3 times, most recently from 6028656 to ef316ff Compare November 1, 2021 13:35
@sv2518 sv2518 force-pushed the sv/local-preconditioners branch 2 times, most recently from aa7584b to 2327850 Compare November 2, 2021 09:37
@sv2518
Copy link
Contributor Author

sv2518 commented Nov 2, 2021

Okay I only need to clean my last few commits and then this is good to go? Thanks for the review @wence- and @dham!

…thin the HyrbidizationPC. The local preconditioner options include either a) jacobi or b) a custom operator supplied by the user.
@sv2518 sv2518 force-pushed the sv/local-preconditioners branch from 8ef78ab to e9990b1 Compare November 2, 2021 14:34
@sv2518
Copy link
Contributor Author

sv2518 commented Nov 3, 2021

Are there any other issues or can we merge? I am pressing on this because a merge would make my work of preparing another PR easier.

Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, go ahead!

@sv2518
Copy link
Contributor Author

sv2518 commented Nov 3, 2021

Ok cool, thanks! This is 5 commits behind master by now, do I need to rebase before I merge?

@wence-
Copy link
Contributor

wence- commented Nov 3, 2021

Ok cool, thanks! This is 5 commits behind master by now, do I need to rebase before I merge?

No, since the merge is clean you can just hit the button.

@sv2518 sv2518 merged commit 4afac04 into master Nov 4, 2021
@sv2518 sv2518 deleted the sv/local-preconditioners branch November 4, 2021 08:20
@sv2518 sv2518 mentioned this pull request Dec 2, 2021
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.

3 participants