Oceananigans.jl
Oceananigans.jl copied to clipboard
Fix `BoundaryAdjacentMean`
Addresses concerns in #4183 and bug
I'm no longer sure that we need the type BoundaryAdjacentMean. Can't we construct a Field directly with all of the information it needs, and then call compute! on that field within update_boundary_condition!? What we need is a function boundary_adjacent_mean(side, field) which constructs the appropriate Reduction. getbc then works correctly (I think); for example the needed value is stored in field.data.
A disadvantage is that we need to use regularize_boundary_condition to set it up for users, because users won't have access to field before they build the model.
However, there are some significant advantages:
- More code re-use; no extra type needed.
- The way the code works is easier to understand, provided that you already understand reductions.
- The type is specific to the boundary and its value can be computed without dependencies via
compute!(field). - Because it is a
Field, users can use an output writer to write its value as a diagnostic. It can also participate in additional operations; for example to compute a flux across the boundary. - We can simplify
update_boundary_condition!.
I'm no longer sure that we need the type
BoundaryAdjacentMean. Can't we construct aFielddirectly with all of the information it needs, and then callcompute!on that field withinupdate_boundary_condition!? What we need is a functionboundary_adjacent_mean(side, field)which constructs the appropriate Reduction.getbcthen works correctly (I think); for example the needed value is stored infield.data.A disadvantage is that we need to use
regularize_boundary_conditionto set it up for users, because users won't have access tofieldbefore they build the model.However, there are some significant advantages:
1. More code re-use; no extra type needed. 2. The way the code works is easier to understand, provided that you already understand reductions. 3. The type is specific to the boundary and its value can be computed without dependencies via `compute!(field)`. 4. Because it is a `Field`, users can use an output writer to write its value as a diagnostic. It can also participate in additional operations; for example to compute a flux across the boundary. 5. We can simplify `update_boundary_condition!`.
I think the reason I ended up with this was to avoid using regularize_boundary_condition but that seems like a reasonable solution. I think we would also need to add a method for update_boundary_condition! when the BC has a compute field in the condition.
Ok, so we have to decide which is best: adopt complication of regularize_boundary_condition but simplify what we get out. Probably the main thing is that Field can participate in abstract operations, so if we think that matters we should use it. Also it might be easier to generalize to open boundary velocities that vary in space (eg not a mean over the whole boundary) ?