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

Allow for registering boundary conditions by package #968

Conversation

lroberts36
Copy link
Collaborator

@lroberts36 lroberts36 commented Oct 26, 2023

PR Summary

This PR contains a proposal for adding the ability to register boundary conditions on a per package basis. I am not wedded to this setup, but it was easy to implement and I think provides the functionality that I foresee needing myself. As written, boundary conditions will behave exactly as they used to unless boundary condition functions are registered within a package.

Per package boundary condition registration is performed by adding to the UserBoundaryFunctions vectors:

template <CoordinateDirection DIR, BCSide SIDE>
auto GetMyBC() {
  return [](std::shared_ptr<MeshBlockData<Real>> &rc, bool coarse) -> void {
    // Implementation of BC here
  };
}

std::shared_ptr<StateDescriptor> Initialize(ParameterInput *pin) {
  ...
  using BF = parthenon::BoundaryFace;
  pkg->UserBoundaryFunctions[BF::inner_x1].push_back(GetMyBC<X1DIR, BCSide::Inner>());
  pkg->UserBoundaryFunctions[BF::inner_x2].push_back(GetMyBC<X2DIR, BCSide::Inner>());
  ...
}

At package resolution, all of the boundary conditions functions from different packages are put into a vector of function pointers and when physical boundary conditions are called these functions are just sequentially called after calling Mesh::MeshBndryFnctn. One clear drawback of the current approach is that if multiple user registered boundary conditions update the same variable, its boundary values will be determined by the last boundary function operating on it that is called (i.e. there is no easy way to set the precedence of boundary functions).

PR Checklist

  • Code passes cpplint
  • New features are documented.
  • Adds a test for any bugs fixed. Adds tests for new features.
  • Code is formatted
  • Changes are summarized in CHANGELOG.md
  • CI has been triggered on Darwin for performance regression tests.
  • Docs build
  • (@lanl.gov employees) Update copyright on changed files

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'm a fan of this. This was something I've been wanting to add for some time now and just never sat down and did it. Glad you did it instead. :)

I particularly like the innovation of allowing each package to register multiple boundary conditions, as I think that's a clean way of ensuring all the flexibility that could possibly be needed.

While we're poking about the boundary condition machinery, can we add a function that takes MeshData instead of MeshBlockData even if it just loops over blocks? That way we could put it in an "all MeshData" task list.

src/interface/state_descriptor.hpp Show resolved Hide resolved
src/interface/state_descriptor.cpp Show resolved Hide resolved
src/interface/state_descriptor.hpp Outdated Show resolved Hide resolved
@lroberts36
Copy link
Collaborator Author

@Yurlungur: We already provide MD boundary conditions in boundary_conditions.cpp

TaskStatus ApplyBoundaryConditionsOnCoarseOrFineMD(std::shared_ptr<MeshData<Real>> &pmd,
                                                   bool coarse) {
  for (int b = 0; b < pmd->NumBlocks(); ++b)
    ApplyBoundaryConditionsOnCoarseOrFine(pmd->GetBlockData(b), coarse);
  return TaskStatus::complete;
}

Are you thinking of something different than this?

@Yurlungur
Copy link
Collaborator

@Yurlungur: We already provide MD boundary conditions in boundary_conditions.cpp

TaskStatus ApplyBoundaryConditionsOnCoarseOrFineMD(std::shared_ptr<MeshData<Real>> &pmd,
                                                   bool coarse) {
  for (int b = 0; b < pmd->NumBlocks(); ++b)
    ApplyBoundaryConditionsOnCoarseOrFine(pmd->GetBlockData(b), coarse);
  return TaskStatus::complete;
}

Are you thinking of something different than this?

Nope that's what I was thinking of.I Just forgot it already existed.

Copy link
Collaborator

@pdmullen pdmullen left a comment

Choose a reason for hiding this comment

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

As discussed, I think future extensions might permit better interfacing with user-defined boundary conditions via the problem generator. @bprather (and perhaps @jonahm-LANL) think that this nestles neatly into your design choice here, if the problem generator itself was a unique package.

In the interim, this looks ready to go in to me.

@Yurlungur
Copy link
Collaborator

This has been ready to go for a while. I'm clicking the button.

@Yurlungur Yurlungur merged commit 4076b31 into lroberts36/multigrid-example-update Nov 15, 2023
43 of 44 checks passed
@Yurlungur Yurlungur deleted the lroberts36/extend-boundary-conditions branch November 15, 2023 16:17
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

Successfully merging this pull request may close these issues.

3 participants