Allow arbitrary fluxes across immersed boundaries and test this with AdvectiveForcing#4987
Allow arbitrary fluxes across immersed boundaries and test this with AdvectiveForcing#4987
AdvectiveForcing#4987Conversation
|
Re-centering and updating the discussion around face boundary conditions for This by itself doesn't seem to work. For example, in the example below, nonhydrostatic_tracer_circle_2d_compare.mp4@glwagner did I miss something about your suggestion? |
You didn't miss anything. This reveals that there is more work to do. Try commenting out these specializations for immersed boundary grid: Oceananigans.jl/src/Advection/immersed_advective_fluxes.jl Lines 57 to 59 in d23257c these seem to hardcode impenetrability for an immersed boundary grid. @simone-silvestri may want to comment on this. I think we have discussed about how the conditional advection operators are redundant with masking. If users want to have open immersed boundaries, we will need to stop using conditional advection operators that assume impenetrability on immersed boundaries. |
|
I guess we could just remove these. The flux on the immersed boundaries will be zeroed by masking the velocities. |
|
This does seem to work. Let's see if removing this doesn't break any tests. |
|
I think the momentum fluxes might require a bit more work, since momentum fluxes always require interpolation of both advecting and advected velocities, and we do not have three-dimensional interpolation operators yet, so the velocity interpolated at the face might not be zero, leading to a non-zero flux across the immersed boundary. |
…ns.jl into tc/unify-adv-forcing-bc
|
@glwagner @simone-silvestri if tests pass this should be ready (they passed locally for me). |
tomchor
changed the title
AdvectiveForcing between immersed boundary and domain boundaryAdvectiveForcing
|
I was looking at this PR again and I still think removing the momentum flux zeroing on immersed boundaries might still be wrong because of the fact that we do not have boundary-aware interpolation. Is this tested? How was it solved? |
Is was tested with the added validation script and the added test, but that's about it. |
|
Would it be possible to add a test for all the advection schemes that should that independently of the internal solution, when computing the momentum flux on peripheral nodes it is zero? |
Tests that all 9 advective momentum flux kernels return zero at immersed peripheral nodes, for Centered(2), UpwindBiased(3), and WENO(5). This documents the expected behavior after PR #4987 removed the explicit conditional_flux zeroing from _advective_momentum_flux_* on ImmersedBoundaryGrid, relying instead on velocity masking in immersed cells and the no-penetration BC at the boundary face. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
@simone-silvestri added #5402 Can you double check if that's what you meant? |
3 participants
This PR makes it possible for users to enforce both open and closed boundaries for
AdvectiveForcingfor both immersed and domain boundaries. This is done by removing methods that enforced zero-flux through immersed boundaries. I'm also adding a test for this and a validation script which produces the following animations for open and closed boundaries, respectively:nonhydrostatic_tracer_circle_2d_compare_open.mp4
nonhydrostatic_tracer_circle_2d_compare.mp4
I also fixed a few errors/typos in other validation scripts.
Importantly, even after this PR, if a user passes aNumbertoAdvectiveForcingthe behavior at domain and immersed boundaries remains different. Therefore this does not close #4812, since aFieldwith proper boundary conditions is required to unify behavior across different boundaries.