Add boundary conditions to the default fields in PrescribedAtmosphere
#387
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #387 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 38 38
Lines 2317 2322 +5
=====================================
- Misses 2317 2322 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return (; rain, snow) | ||
| end | ||
|
|
||
| function default_atmosphere_pressure(grid, times) | ||
| pa = FieldTimeSeries{Center, Center, Nothing}(grid, times) | ||
| bcs = FieldBoundaryConditions(grid, (Center, Center, Nothing)) | ||
| pa = FieldTimeSeries{Center, Center, Nothing}(grid, times; boundary_conditions=bcs) |
There was a problem hiding this comment.
Why don't these boundary conditions get added already? This seems like a change that belongs in Oceananigans. What's the motivation for doing this here? You will have to do this every time you make a FieldTimeSeries right, isn't it a bit much?
There was a problem hiding this comment.
Should we change the default here?
There was a problem hiding this comment.
I guess even if we do, we'd probably don't add defaults that have a Nothing z-location
There was a problem hiding this comment.
@simone-silvestri just for clarification: is this only needed for FieldTimeSeries that are restricted on the surface? Or you are saying that we need to include boundary conditions in the constructors of FieldTimeSeries when we plan to interpolate those time series; this happens to be one example of such case you stumbled upon?
There was a problem hiding this comment.
right, just a one line change in oceananigans needed, as opposed to ~20 lines here
There was a problem hiding this comment.
no @navidcy all thats needed is the default
boundary_conditions = FieldBoundaryConditions(grid, loc)thats the pattern @simone-silvestri uses verbatim
closes #386