materialize `DefaultBoundaryCondition` from bcs in `Field` constructor #4762

simone-silvestri · 2025-09-04T07:04:58Z

this PR should allow the pattern described in #4758, for example:

julia> grid = RectilinearGrid(topology=(Bounded, Bounded, Bounded), size=(4, 4, 4), extent=(1, 1, 1));

julia> boundary_conditions = FieldBoundaryConditions(top = GradientBoundaryCondition(1));

julia> CenterField(grid; boundary_conditions).boundary_conditions
Oceananigans.FieldBoundaryConditions, with boundary conditions
├── west: FluxBoundaryCondition: Nothing
├── east: FluxBoundaryCondition: Nothing
├── south: FluxBoundaryCondition: Nothing
├── north: FluxBoundaryCondition: Nothing
├── bottom: FluxBoundaryCondition: Nothing
├── top: GradientBoundaryCondition: 1.0
└── immersed: Nothing

julia> XFaceField(grid; boundary_conditions).boundary_conditions
Oceananigans.FieldBoundaryConditions, with boundary conditions
├── west: OpenBoundaryCondition{Nothing}: Nothing
├── east: OpenBoundaryCondition{Nothing}: Nothing
├── south: FluxBoundaryCondition: Nothing
├── north: FluxBoundaryCondition: Nothing
├── bottom: FluxBoundaryCondition: Nothing
├── top: GradientBoundaryCondition: 1.0
└── immersed: Nothing
julia> grid = ImmersedBoundaryGrid(grid, GridFittedBottom(-1));

julia> YFaceField(grid; boundary_conditions).boundary_conditions
Oceananigans.FieldBoundaryConditions, with boundary conditions
├── west: FluxBoundaryCondition: Nothing
├── east: FluxBoundaryCondition: Nothing
├── south: OpenBoundaryCondition{Nothing}: Nothing
├── north: OpenBoundaryCondition{Nothing}: Nothing
├── bottom: FluxBoundaryCondition: Nothing
├── top: GradientBoundaryCondition: 1.0
└── immersed: FluxBoundaryCondition: Nothing

closes #4758

…default

simone-silvestri · 2025-09-04T09:24:39Z

I guess we cannot really regularize_boundary_conditions because fields do not have the information of dependencies, but we can add another method that correctly replaces the DefaultBoundaryCondition

…anigans.jl into ss/build-fields-with-default

simone-silvestri · 2025-09-04T11:04:23Z

As part of this PR, I have removed the boundary_condition inside the DefaultBoundaryCondition.
In my opinion, the default is uniquely defined and should depend on location and grid; user-defined BCs should be explicitly specified rather than changing a default globally (which is not even possible because Face and Center locations do not accept the same bcs so this approach could very well lead to error)

glwagner · 2025-09-04T15:46:15Z

src/BoundaryConditions/field_boundary_conditions.jl

-end
-
-DefaultBoundaryCondition() = DefaultBoundaryCondition(NoFluxBoundaryCondition())
+struct DefaultBoundaryCondition end


why did you change this design?

As part of this PR, I have removed the boundary_condition inside the DefaultBoundaryCondition.
In my opinion, the default is uniquely defined and should depend on location and grid; user-defined BCs should be explicitly specified rather than changing a default globally (which is not even possible because Face and Center locations do not accept the same bcs so this approach could very well lead to error)

I thought to remove it for this reason and in particular because validate_boundary_conditions is only grid and location dependent (not model dependent), so it basically forces a limited default choice. But of course, I am open to discuss about it and keep this feature if this is the consensus 😅

I guess, what do you think about models / situations in which "no flux" is irrelevant. For example a model that has no derivatives.

We could have the default be a nothing like on Faces, this would then be general.

That isn't general because models with gradients require NoFluxBoundaryCondition() as a default.

glwagner · 2025-09-04T15:48:47Z

I guess we cannot really regularize_boundary_conditions because fields do not have the information of dependencies, but we can add another method that correctly replaces the DefaultBoundaryCondition

No, this is ok --- only prognostic fields can have field dependencies, and those are built separately inside the model constructor. This PR applies to auxiliary fields only.

glwagner · 2025-09-04T15:51:13Z

src/BoundaryConditions/field_boundary_conditions.jl

+                          north = DefaultBoundaryCondition(),
+                          bottom = DefaultBoundaryCondition(),
+                          top = DefaultBoundaryCondition(),
+                          immersed = DefaultBoundaryCondition()) =


I feel this PR removes a nice feature that I designed for changing the default boundary condition. Note that this code is meant to be applicable to a wide class of models --- not just NonhydrostaticModel and HydrostaticFreeSurfaceModel.

Also, this PR makes the previous code more useful...

Ok, I can keep the default in the DefaultBoundaryCondition. I guess having it or not is not really the scope of this PR

simone-silvestri · 2025-10-06T15:00:36Z

This last commit should have allowed the possibility of choosing a general default without needing to use a field internal to DefaultBoundaryCondition.
The UI remains the same, i.e. on main and on this PR

bcs = FieldBoundaryConditions(FluxBoundaryCondition(10))

will have the same effect

simone-silvestri added 7 commits September 4, 2025 00:35

change this

d64fab4

try like this

4ba486f

regularize

1cafd7a

Merge remote-tracking branch 'origin/main' into ss/build-fields-with-…

4688738

…default

regularize

648e287

regularize also multiregion

9cd7fc3

add fallbacks

fa689f3

materialize

af6ae1a

simone-silvestri changed the title ~~Regularize FieldBoundaryConditions in Field constructor~~ materialize DefaultBoundaryCondition from bcs in Field constructor Sep 4, 2025

simone-silvestri added 6 commits September 4, 2025 11:33

Merge branch 'main' into ss/build-fields-with-default

cc033aa

clean up a bit

b0255f8

Merge branch 'ss/build-fields-with-default' of github.com:CliMA/Ocean…

14b3814

…anigans.jl into ss/build-fields-with-default

materialize do not regularize

c997c93

Update regularize_immersed_boundary_condition logic

d96a8f0

send it

c1874e0

Fix case sensitivity for missing boundary condition

98ce374

glwagner reviewed Sep 4, 2025

View reviewed changes

default

66bf31b

simone-silvestri added 7 commits October 6, 2025 17:00

Merge branch 'main' into ss/build-fields-with-default

8a992fc

bugfix

39dc07f

default_prognostic_bc

2002020

add bc

de2bc90

this should work

2684601

ok this should cover all the cases

bc2755a

bugfix

d411608

simone-silvestri requested a review from navidcy October 8, 2025 07:01

Merge branch 'main' into ss/build-fields-with-default

b61194c

materialize DefaultBoundaryCondition from bcs in Field constructor #4762

Are you sure you want to change the base?

materialize DefaultBoundaryCondition from bcs in Field constructor #4762

Conversation

simone-silvestri commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simone-silvestri commented Sep 4, 2025

Uh oh!

simone-silvestri commented Sep 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simone-silvestri Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glwagner commented Sep 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simone-silvestri commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

materialize `DefaultBoundaryCondition` from bcs in `Field` constructor #4762

materialize `DefaultBoundaryCondition` from bcs in `Field` constructor #4762

simone-silvestri commented Sep 4, 2025 •

edited

Loading

simone-silvestri Sep 5, 2025 •

edited

Loading