ginkgo-project / ginkgo

Numerical linear algebra software package
https://ginkgo-project.github.io/
BSD 3-Clause "New" or "Revised" License
384 stars 86 forks source link

Update defaults for distributed multigrid #1612

Closed pratikvn closed 1 month ago

pratikvn commented 1 month ago

This PR updates the defaults for distributed multigrid for the smoother and the coarse solver. It also adds a test for the distributed pgm preconditioned solver

MarcelKoch commented 1 month ago

@pratikvn I will fix the CI issues to push this for the release.

MarcelKoch commented 1 month ago

One thing to note is that the added tests are EXTREMELY slow. On nla-gpu the preconditioned test are 100x slower than the unpreconditioned tests. But I don't think this PR should fix performance issues, so I will just leave it as it is.

upsj commented 1 month ago

slow as in long runtime or as in slow convergence (or both)? In any case, I think we should reconsider whether to merge this PR in this state then

MarcelKoch commented 1 month ago

slow as in long runtime or as in slow convergence (or both)? In any case, I think we should reconsider whether to merge this PR in this state then

definitely runtime, but I think the number of iterations is also quite bad.

IMO this PR should still continue, since otherwise the user will get segfaults and the like. Even sequentially our mg is not very good, so it wouldn't be much of a difference to that.

yhmtsai commented 1 month ago

The slow convergence is expected in the 3pt stencil because the PGM does not handle the equal-weight matrix well currently. The corasening matrix might almost like the original matrix (n -> n - 10 for example)

upsj commented 1 month ago

Alright, then I withdraw my objection ;) that's an issue we should definitely tackle soon though. I think with some deterministic or randomized tie-breaking we should be able to handle that, create at least a 25%-33% reduction per level (haven't thought the worst case through entirely)

pratikvn commented 1 month ago

Given that we are testing that the solver + preconditioner work, I think we should just set the factory parameter with_deterministic(false) for the PGM factory. That should speedup the convergence.

MarcelKoch commented 1 month ago

Given that we are testing that the solver + preconditioner work, I think we should just set the factory parameter with_deterministic(false) for the PGM factory. That should speedup the convergence.

Can't confirm that this has any effect (on nla-gpu).

sonarcloud[bot] commented 1 month ago

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 65.11628% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 89.97%. Comparing base (5b18510) to head (1fc3d7d). Report is 52 commits behind head on develop.

Files Patch % Lines
test/matrix/csr_kernels2.cpp 0.00% 11 Missing :warning:
core/solver/multigrid.cpp 50.00% 3 Missing :warning:
test/mpi/solver/solver.cpp 95.83% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #1612 +/- ## =========================================== - Coverage 90.01% 89.97% -0.04% =========================================== Files 736 752 +16 Lines 59831 60445 +614 =========================================== + Hits 53857 54387 +530 - Misses 5974 6058 +84 ```

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