Do not call BC on coarse buffer if there are no neighbors to fill it#1365
Do not call BC on coarse buffer if there are no neighbors to fill it#1365lroberts36 merged 14 commits intodevelopfrom
Conversation
…r level to fill it
adamdempsey90
requested review from
Yurlungur,
lroberts36,
pdmullen and
pgrete
|
Won't the coarse buffer always be filled with zeros in this case since kokkos views are zero-initialized? What should it be filled with? Or is the issue that your EOS calls happen inside the boundary call? |
Yeah, the coarse buffer will still be filled with zeros but we will not call the actual boundary kernel that uses it, so it doesn't matter. The BC in question was an extrapolation BC that needed to do an EOS call to set sie from the extrapolated rho and T. |
|
@lroberts36 to add additional review |
This prevents undefined behavior from uninitialized variables
Keep PR focused on coarser neighbor optimization
| // We only need to call the BC on the coarse buffer if one of the neighbors | ||
| // is at a coarser level than us. | ||
| if (coarse) { | ||
| bool has_coarser_neighbor = false; | ||
| const int mylevel = pmb->loc.level(); | ||
| for (const auto &nb : pmb->neighbors) { | ||
| if (nb.origin_loc.level() < mylevel) { | ||
| has_coarser_neighbor = true; | ||
| break; | ||
| } | ||
| } | ||
| if (!has_coarser_neighbor) return TaskStatus::complete; | ||
| } | ||
|
|
There was a problem hiding this comment.
@adamdempsey90: This is definitely correct. Nevertheless, I would suggest we add this logic to MeshBlock rather than adding it here and just call pmb->HasCoarserNeighbor(); here. I put up another PR based on this one (#1367) that adds a function MeshBlock::HasCoarserNeighbor(), sets the return value of that once during neighbor finding, and calls that function here and another place we do a similar check. I also refactored how we access neighbors in MeshBlock there, it is really not directly related to this change but this was a good place to make the access patterns a little safer. Please take a look and let me know or just merge that PR into this one. I will approve this PR after that is in (or if we decide not to include it).
3 participants
PR Summary
Ran into this issue in Artemis when calling one of our BCs with the coarse flag set to true in a situation where there was no neighbor at a coarser level. In that scenario, the coarse buffer was filled entirely with zeros since there was no one to fill it. Our EOS caught it since we were then passing in hard 0 density and energy values.
This adds a check for coarser neighbors before calling the BCs on the coarse buffer.
PR Checklist