-
Notifications
You must be signed in to change notification settings - Fork 2
Compute salinity for the bottom boundary conditions at the top of the grid #73
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #73 +/- ##
=======================================
Coverage 70.44% 70.44%
=======================================
Files 27 27
Lines 829 829
=======================================
Hits 584 584
Misses 245 245 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -34,7 +34,8 @@ IceWaterThermalEquilibrium(; salinity=0) = IceWaterThermalEquilibrium(salinity) | |||
@inline bottom_temperature(i, j, grid, bc::PrescribedTemperature{<:Number}, args...) = bc.temperature | |||
|
|||
@inline function bottom_temperature(i, j, grid, bc::IceWaterThermalEquilibrium, liquidus) | |||
Sₒ = get_tracer(i, j, 1, grid, bc.salinity) | |||
kᴺ = size(grid, 3) | |||
Sₒ = get_tracer(i, j, kᴺ, grid, bc.salinity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so grid
is the ocean grid or the sea ice grid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confusing to find this inside ClimaSeaIce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is ocean salinity, so logically this should be the ocean's grid. However, in this case, we are passing to the bottom_temperature
function the sea ice grid. Luckily, there is no ambiguity in case of a slab thermodynamic model as we have decided in #68 to have the momentum equations (and by extension any 2D field which in principle has no k-location) live at the top of the grid to comply with bathymetry and a possible coupled ocean. So if this function is used in the slab model (2D) it should be kᴺ = size(grid, 3)
and not k = 1
.
We still have to think about a strategy for a possible multi-layer model (is the k-index the index of the layers? Do we add another dimension?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel we likely want a 3D grid for multi-layer sea ice, because we need that for 3D fields and 3D kernels (though many of the kernels will be 2D)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I guess we can delay this until we decide the strategy for multi-layer sea ice. In the meantime, in ClimaOcean we can pass the ocean salinity to the boundary condition as
interior(S, :, :, grid.Nz : grid.Nz)
instead of
view(S, :, :, grid.Nz)
to make sure the indexing is correct.
Supposedly, this is the salinity of the surface of the ocean. Given the convention that a slab sea ice is at the top of the grid, we need to compute the salinity at
size(grid, 3)
instead of1