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

Should reductions exclude peripheral_node? #3064

Closed
glwagner opened this issue Apr 13, 2023 · 2 comments · Fixed by #3122
Closed

Should reductions exclude peripheral_node? #3064

glwagner opened this issue Apr 13, 2023 · 2 comments · Fixed by #3122
Labels
immersed boundaries ⛰️ Less Ocean, more anigans question 💭 No such thing as a stupid question

Comments

@glwagner
Copy link
Member

Right now reductions only exclude immersed peripheral nodes:

return get_condition(condition.func, i, j, k, ibg, args...) & !(immersed_peripheral_node(i, j, k, ibg, LX(), LY(), LZ()))

It recently caused me a lot of pain and confusion and time that "ordinary" peripheral nodes are included in the reduction, but immersed peripheral nodes are not.

What is the logic for treating immersed boundaries differently from ordinary boundaries? I think we should either exclude only inactive nodes or peripheral nodes, but this behavior should be consistent between immersed and not immersed grids.

This change would mean we don't need special reductions (at least for fields that are not reduced) on immersed vs not-immersed grids.

@simone-silvestri may have the answer.

@glwagner glwagner added question 💭 No such thing as a stupid question immersed boundaries ⛰️ Less Ocean, more anigans labels Apr 13, 2023
@glwagner
Copy link
Member Author

glwagner commented Apr 13, 2023

Simple example why this doesn't make sense: immersed_peripheral_node is

@inline immersed_peripheral_node(i, j, k, ibg::IBG, LX, LY, LZ) = peripheral_node(i, j, k, ibg, LX, LY, LZ) &
!peripheral_node(i, j, k, ibg.underlying_grid, LX, LY, LZ)

So, for example, if the entire bottom of a grid is immersed, then it will be included in the reduction --- because those nodes are on the periphery of the underlying grid, so

!peripheral_node(i, j, k, ibg.underlying_grid, LX, LY, LZ)

is false. Including the bottom row of cells in the reduction makes no sense because these nodes are far beneath the immersed boundary.

As a quick fix we could add another condition that also ignores inactive_node.

But I'd also like to understand why we don't simply ignore all peripheral_nodes...

@navidcy
Copy link
Collaborator

navidcy commented Apr 13, 2023

cc @simone-silvestri

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
immersed boundaries ⛰️ Less Ocean, more anigans question 💭 No such thing as a stupid question
Projects
None yet
2 participants