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

Paper #74

Closed
PetrKryslUCSD opened this issue Sep 24, 2024 · 6 comments
Closed

Paper #74

PetrKryslUCSD opened this issue Sep 24, 2024 · 6 comments

Comments

@PetrKryslUCSD
Copy link

Line 21: uniquely -- is it used to indicate "solely"?
Line 70: patch-based -- not sure what it means. It does not appear to be "standard" terminology.
Line 82, 83: Shur -> Schur
Line 84: it's spectral -> its spectral
Line 88: The mass matrix is approximated by a Conjugate Gradient (CG) solver with... This sentence needs some work. I am not sure what you mean.
Line 91: it is setup -> it is set up
Line 128 - 131: h(div) -> H(div), h(curl) -> H(curl)

@JordiManyer
Copy link
Member

JordiManyer commented Sep 25, 2024

I will address your semantic/grammar related comments.
Concerning the rest:

Line 70: patch-based -- not sure what it means. It does not appear to be "standard" terminology.

I agree, it is not the most standard terminology. We are basically addressing the same smoothers described here.
"Patch-based" refers to topologically-generated patches of cells/faces/edges/nodes where we build overlapping functional spaces to create smoothing operators.
Would their terminology of topological construction of space decompositions for multigrid relaxation methods be suitable? How can I make this clearer?

Line 88: The mass matrix is approximated by a Conjugate Gradient (CG) solver with... This sentence needs some work. I am not sure what you mean.

In this sentence, approximated is usually used in the literature as a way of saying solve using an inexact solver instead of a direct method. But I agree it's not the clearest. The idea is that our block-preconditioner has two diagonal blocks we somehow need to invert/solve at each iteration. The pressure block we solve using a CG+Jacobi, whereas the velocity block we solve using a GMG.

Maybe we could rephrase it as:

"""
The diagonal blocks have to be solved at each iteration. The mass matrix is solved by a Conjugate Gradient (CG) solver with Jacobi preconditioner. The eliminated velocity block is solved by a 2-level V-cycle GMG solver, where the coarsest level is inverted exactly in a single processor.
"""

@PetrKryslUCSD
Copy link
Author

Patch based: maybe you could just add that explanation of what those patches are (essentially what you are saying above)?

The blocked system: maybe one could refer to solves with coefficient matrices (a) mass matrix and (b) solve with a velocity matrix, the first one with CG+Jacobi, the second with GMG?

@JordiManyer
Copy link
Member

I've hopefully addressed everything. You can find the new compiled version of the pdf here. Let me know if there is anything that needs to be looked at.

@Leticia-maria
Copy link

Leticia-maria commented Sep 25, 2024

Hello, I have some points about your paper:

  • Review reference syntax

🟡 SKIP DOIs

- No DOI given, and none found for title: PETSc/TAO Users Manual
- No DOI given, and none found for title: MPI: A Message-Passing Interface Standard Version ...
- No DOI given, and none found for title: GridapPETSc
- No DOI given, and none found for title: GridapP4est
- No DOI given, and none found for title: PartitionedArrays
- No DOI given, and none found for title: The Trilinos Project Website
- No DOI given, and none found for title: \sl hypre: High Performance Preconditioners

(I think it is okay, just double check to make sure you have not missed any DOIs)

  • More details on how the benchmark was performed: details about which benchmark method was used. It was not clear if the algorithm is compute-bound or memory-bound, maybe expand a little bit more on that.
  • Overall, good propositions, I think it fits into the journal.

@JordiManyer
Copy link
Member

Those skipped dois are all repos/codes. I tried to find dois for them, but to my best knowledge there is none.

@Leticia-maria
Copy link

Those skipped dois are all repos/codes. I tried to find dois for them, but to my best knowledge there is none.

That sounds good (I actually assumed so, I just wanted to make sure you haven't missed anything)

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

No branches or pull requests

4 participants