[WIP] Implement weighted BFBT with different discretization for (BC^{-1}B^T).#6440
Open
quangx wants to merge 4 commits intogeodynamics:mainfrom
Open
[WIP] Implement weighted BFBT with different discretization for (BC^{-1}B^T).#6440quangx wants to merge 4 commits intogeodynamics:mainfrom
quangx wants to merge 4 commits intogeodynamics:mainfrom
Conversation
bangerth
reviewed
Jun 15, 2025
Contributor
bangerth
left a comment
bangerth
left a comment
There was a problem hiding this comment.
So when you do this, what happens?
source/simulator/solver.cc
Outdated
Comment on lines
388
to
399
| }; | ||
| template<typename Range, | ||
| typename Domain, | ||
| typename Payload> | ||
| LinearOperator<Range, Domain, Payload> diag_operator(LinearOperator<Range,Domain,Payload> &exemplar, const TrilinosWrappers::MPI::Vector &diagonal) |
Contributor
There was a problem hiding this comment.
Add 3 empty lines and then also add documentation for this function.
source/simulator/solver.cc
Outdated
Comment on lines
474
to
507
| solver.solve(mp_matrix, | ||
| solver.solve(matrix, |
Contributor
There was a problem hiding this comment.
Do you even still need mp_matrix in that case?
Contributor
Author
|
I have tried address the changes above (except removing the mp_matrix - will get to it). Probably not clear from the rebase, sorry. Do not review quite yet. |
bangerth
requested changes
Jun 17, 2025
Comment on lines
169
to
174
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| /** |
Contributor
There was a problem hiding this comment.
We typically have 3 empty lines between top-level constructs. So take one off again here.
Comment on lines
391
to
396
| /** | ||
| * Given a diagonal matrix stored as a vector, | ||
| * create an operator that represents its action. | ||
| */ | ||
|
|
||
| template<typename Range, |
Contributor
There was a problem hiding this comment.
Suggested change
| /** | |
| * Given a diagonal matrix stored as a vector, | |
| * create an operator that represents its action. | |
| */ | |
| template<typename Range, | |
| /** | |
| * Given a diagonal matrix stored as a vector, | |
| * create an operator that represents its action. | |
| */ | |
| template<typename Range, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This implementation applies (BC^{-1}B^T)^{-1}) by creating an operator that applies B^T, followed by C^{-1},
followed by B, then doing a CG solve with this new operator.
@tjhei