Skip to content

Conversation

@lroberts36
Copy link
Collaborator

@lroberts36 lroberts36 commented Sep 3, 2025

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 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
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
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.

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.

@lroberts36
Copy link
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
Copy link
Collaborator

Yurlungur commented Sep 4, 2025

👍 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
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.

@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
Copy link
Collaborator Author

lroberts36 commented Sep 8, 2025

@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
Copy link
Collaborator

pdmullen commented Sep 8, 2025

@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.

4 participants