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

MG performance upgrades #973

Merged
merged 43 commits into from
Mar 19, 2024
Merged

MG performance upgrades #973

merged 43 commits into from
Mar 19, 2024

Conversation

lroberts36
Copy link
Collaborator

@lroberts36 lroberts36 commented Nov 8, 2023

PR Summary

This PR makes some changes to make things go faster, including:

  1. Caches PackDescriptors for boundary conditions generic using a map. At least in MG, re-creating pack descriptors every call was a significant performance hit.
  2. Re-arranges internal fields created by solvers to minimize the number of fields that share ghost data.
  3. Adds mesh data for MG communication that only includes internal fields (so that fields from user packages are not communicated unnecessarily during MG solves).
  4. Adds hierarchical parallelism in some of the MG utilities.
  5. Adds a setup phase for MG to reduce the number of fields that are restricted.
  6. Adds some solver parameters.

This breaks downstream codes that use a solver, since now solvers require a call to AddSetupTasks before calling AddTasks.

PR Checklist

  • Code passes cpplint
  • Code is formatted
  • Changes are summarized in CHANGELOG.md
  • CI has been triggered on Darwin for performance regression tests.
  • (@lanl.gov employees) Update copyright on changed files

@lroberts36 lroberts36 changed the base branch from lroberts36/multigrid-example-update to lroberts36/extend-boundary-conditions November 8, 2023 01:39
Base automatically changed from lroberts36/extend-boundary-conditions to lroberts36/multigrid-example-update November 15, 2023 16:17
@lroberts36 lroberts36 changed the base branch from lroberts36/multigrid-example-update to develop November 27, 2023 21:20
@lroberts36 lroberts36 changed the title WIP: MG performance upgrades MG performance upgrades Mar 11, 2024
@lroberts36 lroberts36 requested review from jdolence, Yurlungur, bprather and pgrete and removed request for jdolence March 11, 2024 23:27
Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be useful to better document the solver parameters and how to use the solver more generically. But overall this LGTM.

src/bvals/boundary_conditions_generic.hpp Show resolved Hide resolved
src/solvers/bicgstab_solver.hpp Show resolved Hide resolved
@lroberts36
Copy link
Collaborator Author

I think it would be useful to better document the solver parameters and how to use the solver more generically.

Agreed, but I am not sure that the dust has settled on the solver stuff yet. I mostly want to get these changes in now so the branch doesn't go stale.

src/interface/sparse_pack.hpp Show resolved Hide resolved
src/solvers/bicgstab_solver.hpp Show resolved Hide resolved
src/solvers/solver_utils.hpp Show resolved Hide resolved
src/solvers/solver_utils.hpp Show resolved Hide resolved
Copy link
Collaborator

@pgrete pgrete left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of clarifying questions, so feel free to merge when ready.

Otherwise, I second @Yurlungur comment on documentation ;)

src/solvers/bicgstab_solver.hpp Show resolved Hide resolved
src/solvers/mg_solver.hpp Outdated Show resolved Hide resolved
src/solvers/mg_solver.hpp Outdated Show resolved Hide resolved
src/solvers/mg_solver.hpp Outdated Show resolved Hide resolved
src/solvers/solver_utils.hpp Outdated Show resolved Hide resolved
src/solvers/solver_utils.hpp Outdated Show resolved Hide resolved
@lroberts36 lroberts36 merged commit d7127c7 into develop Mar 19, 2024
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants