Coarse level options for AMG preconditioner

sseraj commented 5 months ago

Purpose

This PR adds separate ASM overlap, ILU fill level, and inner (ILU) iteration options for the coarse levels when using the AMG preconditioner. The motivation behind this is to allow for cheaper iterations at the coarse levels to speed up the overall solution time. The defaults for the coarse levels are set to the cheapest options possible (no ASM overlap, no ILU fill, 1 inner iteration). This changes the default behavior but should be advantageous for most cases.

Expected time until merged

2-3 weeks

Type of change

[ ] Bugfix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (non-backwards-compatible fix or feature)
[ ] Code style update (formatting, renaming)
[ ] Refactoring (no functional changes, no API changes)
[ ] Documentation update
[ ] Maintenance update
[ ] Other (please describe)

Testing

I added an AMG test with non-default options. The default options are used in the existing tests.

Checklist

[x] I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
[x] I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
[x] I have run unit and regression tests which pass locally with my changes
[x] I have added new tests that prove my fix is effective or that my feature works
[x] I have added necessary documentation

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 41.02%. Comparing base (7de267b) to head (beb06fc). Report is 2 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #359 +/- ## ======================================= Coverage 41.02% 41.02% ======================================= Files 13 13 Lines 4119 4119 ======================================= Hits 1690 1690 Misses 2429 2429 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

anilyil commented 4 months ago

After reviewing this a bit more, I noticed that we do not have the option to add iterative refinement with the AMG PC. Even though NK and ANK solvers default to no refinement, would it make sense to add a similar block like:

        ! Set the orthogonalization method for GMRES
        select case (GMRESOrthogType)
        case ('modified_gram_schmidt')
            ! Use modified Gram-Schmidt
            call KSPGMRESSetOrthogonalization(kspObject, KSPGMRESModifiedGramSchmidtOrthogonalization, ierr)
        case ('cgs_never_refine')
            ! Use classical Gram-Schmidt with no refinement
            call KSPGMRESSetCGSRefinementType(kspObject, KSP_GMRES_CGS_REFINE_NEVER, ierr)
        case ('cgs_refine_if_needed')
            ! Use classical Gram-Schmidt with refinement if needed
            call KSPGMRESSetCGSRefinementType(kspObject, KSP_GMRES_CGS_REFINE_IFNEEDED, ierr)
        case ('cgs_always_refine')
            ! Use classical Gram-Schmidt with refinement at every iteration
            call KSPGMRESSetCGSRefinementType(kspObject, KSP_GMRES_CGS_REFINE_ALWAYS, ierr)
        end select
        call EChk(ierr, __FILE__, __LINE__)

to the AMG PC setup?

anilyil commented 4 months ago

The new changes look good. I also want to add the option to enable iterative refinement to ANK and NK if desired. Default should be no refinement as that is what the code is doing for all cases now.

I want to add 2 python options that control this behavior: ANKUseGMRESRefinement and NKUseGMRESRefinement or we can replace GMRES with "Orthog" or something along those lines. Then these options can be added around the code where ANK and NK disables refinement (e.g. here for ANK: https://github.com/mdolab/adflow/blob/main/src/NKSolver/NKSolvers.F90#L2027-L2029)

Should we make this change in this PR or should I create a new PR for this? I can also make these changes here and push. Up to you @sseraj.

sseraj commented 4 months ago

I think it would be better to make the ANK and NK changes in a separate PR

lamkina commented 3 months ago

@eirikurj I think this is good to go, but we can wait to merge if you want to take a look.

mdolab / adflow