Add distributed preconditioners to benchmarks #1845
Conversation
pratikvn
added
1:ST:ready-for-review
There was a problem hiding this comment.
The changes are mostly fine (I would want to see the mg also for non-distributed), but there are two more levels of generality we could achieve:
- define the inner schwarz preconditioner through a separate option;
- define everything directly through our json format.
I think 2. is definitely not in the scope of this PR. For 1., however, I could see this getting added.
| "parilut-isai, ic-isai, ilu-isai, sor, overhead"); | ||
| "parilut-isai, ic-isai, ilu-isai, sor, overhead" | ||
| #ifdef GINKGO_BUILD_MPI | ||
| ", schwarz-jacobi, schwarz-ilu, schwarz-ic, schwarz-lu, dist-mg" |
There was a problem hiding this comment.
Shouldn't mg just be its own thing? You can use it also without mpi.
| "parilut-isai, ic-isai, ilu-isai, sor, overhead"); | ||
| "parilut-isai, ic-isai, ilu-isai, sor, overhead" | ||
| #ifdef GINKGO_BUILD_MPI | ||
| ", schwarz-jacobi, schwarz-ilu, schwarz-ic, schwarz-lu, dist-mg" |
There was a problem hiding this comment.
IMO it should just be schwarz and then another option to set the local preconditioner for schwarz.
| DEFINE_uint32( | ||
| mg_max_num_levels, false, | ||
| "The maximum number of levels to use for the Multigrid preconditioner"); | ||
|
|
||
| DEFINE_double(mg_tolerance, false, "The tolerance for the coarse solver"); | ||
|
|
||
| DEFINE_uint32(mg_max_iters, false, | ||
| "The max number of iterations for the coarse solver"); |
There was a problem hiding this comment.
To me, this looks like we reached the limit of what we can do through the CLI. I think for now this is fine, but we should consider in the long term to also allow our json config to define these parameters.
This PR adds distributed preconditioners to benchmarks.
Extracted from: #1660