Skip to content

Allow for user overridable boundary conditions - #1314

Merged
lroberts36 merged 10 commits into
developfrom
lroberts36/add-user-boundary-override
Sep 11, 2025
Merged

Allow for user overridable boundary conditions#1314
lroberts36 merged 10 commits into
developfrom
lroberts36/add-user-boundary-override

Conversation

@lroberts36

@lroberts36 lroberts36 commented Sep 3, 2025

Copy link
Copy Markdown
Collaborator

PR Summary

Add the option for injecting user specified boundary conditions in AddBoundaryExchangeTasks. These changes were duplicated in #1250 and #1236, so it seemed better to make a single small PR. Additionally, it switches to a task list for boundary communication in mesh.cpp.

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
  • Change is breaking (API, behavior, ...)
    • Change is additionally added to CHANGELOG.md in the breaking section
    • PR is marked as breaking
    • Short summary API changes at the top of the PR (plus optionally with an automated update/fix script)
  • CI has been triggered on Darwin for performance regression tests.
  • Docs build
  • (@lanl.gov employees) Update copyright on changed files

Comment thread src/bvals/comms/bvals_in_one.hpp Outdated
Comment on lines +111 to +115
// Need to prolongate internal bounds after setting physical boundary
// conditions, since the internal prolongation uses the values of the
// normal buffer on shared elements (rather than values in the coarse)
// buffer for prolongation.
fbound = tl.AddTask(fbound, TF(ProlongateInternalBounds<bounds>), md);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This piece is new and is required for non-cell centered fields.

@lroberts36
lroberts36 requested review from Yurlungur, pdmullen and pgrete and removed request for Yurlungur and pgrete September 3, 2025 22:57

@Yurlungur Yurlungur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I understand correctly, this is adding the ability to have AddBoundaryExchangeTasks have an arbitrary boundary condition function... Is this only useful for the multigrid stuff? It might be good to document it.

Comment thread src/mesh/mesh.cpp
@lroberts36

Copy link
Copy Markdown
Collaborator Author

If I understand correctly, this is adding the ability to have AddBoundaryExchangeTasks have an arbitrary boundary condition function... Is this only useful for the multigrid stuff? It might be good to document it.

@Yurlungur: It is useful for multigrid (e.g. if you want to apply a different boundary conditions on the same variable in different containers) and it was also useful for the forest of octrees stuff (hence why I made it its own PR).

@Yurlungur

Yurlungur commented Sep 4, 2025

Copy link
Copy Markdown
Collaborator

👍 I think adding a sentence to the docs would be useful. Otherwise LGTM.

@lroberts36 lroberts36 changed the title Allow for user overridable boundary conditions WIP: Allow for user overridable boundary conditions Sep 4, 2025
@lroberts36 lroberts36 changed the title WIP: Allow for user overridable boundary conditions Allow for user overridable boundary conditions Sep 4, 2025
@lroberts36
lroberts36 changed the base branch from develop to lroberts36/non-cartesian-solvers September 4, 2025 19:20
Base automatically changed from lroberts36/non-cartesian-solvers to develop September 5, 2025 05:58

@pdmullen pdmullen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@lroberts36 Hoping you can help me better understand this MR:

I originally thought this might be an additional API for enrolling custom boundary conditions... for example, if I had a boundary condition MyCustomBCPackageA that operates on a MeshData subset containing only variables from PackageA, then it might be useful to call AddBoundaryExchangeTasks(md_subset, old_args..., MyCustomBCPackageA). This might be nice so that I can avoid having checks in my custom boundary condition to see which meshdata subset register I am currently working with. However, looking at this PR's companion PR #1315, it looks like this new utility is instead being used as a selector between e.g., SetBoundary and ApplyBoundaryConditionsOnCoarseOrFineMD . Furthermore, since our custom boundary conditions typically are specific to e.g., InnerX1, I guess this new utility is not really for that... do I understand correctly? As such, I then wanted to confirm that independent of what function I pass to AddBoundaryExchangeTasks, that logical BCs are still always applied.

@lroberts36

lroberts36 commented Sep 8, 2025

Copy link
Copy Markdown
Collaborator Author

@lroberts36 Hoping you can help me better understand this MR:

I originally thought this might be an additional API for enrolling custom boundary conditions... for example, if I had a boundary condition MyCustomBCPackageA that operates on a MeshData subset containing only variables from PackageA, then it might be useful to call AddBoundaryExchangeTasks(md_subset, old_args..., MyCustomBCPackageA). This might be nice so that I can avoid having checks in my custom boundary condition to see which meshdata subset register I am currently working with. However, looking at this PR's companion PR #1315, it looks like this new utility is instead being used as a selector between e.g., SetBoundary and ApplyBoundaryConditionsOnCoarseOrFineMD . Furthermore, since our custom boundary conditions typically are specific to e.g., InnerX1, I guess this new utility is not really for that... do I understand correctly? As such, I then wanted to confirm that independent of what function I pass to AddBoundaryExchangeTasks, that logical BCs are still always applied.

@pdmullen: All this PR does is allow you to override ApplyBoundaryConditionsOnCoarseOrFineMD when calling AddBoundaryExchangeTasks with some user defined task(s). The user is responsible for making sure that the new task(s) actually set all the physical boundaries, deal with the coarse/fine flag, etc. It would be straightforward to add a wrapper that takes a BC of the form

template <CoordinateDirection DIR, BCSide SIDE>
void MyPackageBC(std::shared_ptr<MeshBlockData<Real>> &rc, bool coarse)

and goes over the required template parameters and blocks.

The interior ghost communication is unchanged by this PR.

@pdmullen

pdmullen commented Sep 8, 2025

Copy link
Copy Markdown
Collaborator

@lroberts36 Hoping you can help me better understand this MR:
I originally thought this might be an additional API for enrolling custom boundary conditions... for example, if I had a boundary condition MyCustomBCPackageA that operates on a MeshData subset containing only variables from PackageA, then it might be useful to call AddBoundaryExchangeTasks(md_subset, old_args..., MyCustomBCPackageA). This might be nice so that I can avoid having checks in my custom boundary condition to see which meshdata subset register I am currently working with. However, looking at this PR's companion PR #1315, it looks like this new utility is instead being used as a selector between e.g., SetBoundary and ApplyBoundaryConditionsOnCoarseOrFineMD . Furthermore, since our custom boundary conditions typically are specific to e.g., InnerX1, I guess this new utility is not really for that... do I understand correctly? As such, I then wanted to confirm that independent of what function I pass to AddBoundaryExchangeTasks, that logical BCs are still always applied.

@pdmullen: All this PR does is allow you to override ApplyBoundaryConditionsOnCoarseOrFineMD when calling AddBoundaryExchangeTasks with some user defined task(s). The user is responsible for making sure that the new task(s) actually set all the physical boundaries, deal with the coarse/fine flag, etc. It would be straightforward to add a wrapper that takes a BC of the form

template <CoordinateDirection DIR, BCSide SIDE>
void MyPackageBC(std::shared_ptr<MeshBlockData<Real>> &rc, bool coarse)

and goes over the required template parameters and blocks.

The interior ghost communication is unchanged by this PR.

Got it! 😄 Thanks! I had a disconnect between user-defined BCs as I typically interface with them as opposed to an override to ApplyBoundaryConditionsOnCoarseOrFineMD. Makes sense now.

@lroberts36
lroberts36 disabled auto-merge September 11, 2025 21:19
@lroberts36
lroberts36 merged commit 4720d13 into develop Sep 11, 2025
37 checks passed
@lroberts36
lroberts36 deleted the lroberts36/add-user-boundary-override branch September 11, 2025 23:53
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