-
Notifications
You must be signed in to change notification settings - Fork 40
Allow for user overridable boundary conditions #1314
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
Conversation
src/bvals/comms/bvals_in_one.hpp
Outdated
| // 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); |
There was a problem hiding this comment.
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.
…ser-boundary-override
There was a problem hiding this 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.
@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). |
|
👍 I think adding a sentence to the docs would be useful. Otherwise LGTM. |
There was a problem hiding this 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.
@pdmullen: All this PR does is allow you to override 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 |
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 inmesh.cpp.PR Checklist