Issue1088 slab ocean bug - #1105
Merged
Merged
Conversation
The salt flux is already calculated in mergeFluxes of IOceanBoundary. In this PR I use this in SlabOcean to calculate the change in SSS, instead of the salt flux derived from freshwater flux.
We have to define a local relaxation timescale anyway, so we might as well use its reciprocal and do repeated multiplications instead of divisions. This is probably an inperceptible speedup, but it's nice that we don't have local variables shaddowing global ones.
It's not needed unless the cell fluxes out in a single time step and if that happens, the problem is somewhere else!
Change era5_topaz4_maker.py so that ocean velocities are zero on land (as we have always done before).
Just follow suggestions by ruff. Nothing fancy, but it does give better looking code.
Remove an unused variable.
We remove all ice in the grid cell when it falls below the minima, cMin or hMin. Doing this, we also have to reset newice to zero - otherwise it may be erroneously used by other routines, e.g. IOceanBoundary::mergeFluxes to calculate freshwater flux.
Adds several const statements and renames variables to use camel case. Clearer program flow w.r.t. updates of snow, concentration, and volume.
It is a tunable parameter in the old neXtSIM, but that doesn't really make sense. In Hibler's paper it's just 2, and it should be a constant.
sFlux is in kg/m^2/s, but we need PSU/m^2/s
Also report index, not just [i,j], when we encounter out-of-bounds values.
These values may be unphysical, but they appear in the first few time steps when the model is "settling". Stopping at those makes the error checking unusable.
We should record the ice and snow volume lost when everything melts away because we're below the cMin or hMin values. Also, be less agressive with using _ft.
Doesn't really belong here ... but it should be done.
Take advantage of the fact that cIce >= cMin if there is ice and that deltaCMelt == 0 if there is no melt.
Assign proper values to deltaHi, botMelt (diagnostic), topMelt (diagnostic), and snowMelt. First and last are used later in the code to compute freshwater fluxes.
For some reason I can't use std::max(cMin, cIce[i]) on my GPU machine. The reason seems to be that cMin and cIce[i] are of different types, but I've tried casting cIce[i] to FloatType and that doesn't help.
Left in some debug code that I didn't want to commit. I've removed it now.
They were the changes in slab thickness, but everything should be volumetric - it makes things easier.
Just use a salt flux to nudge in the numerator of the sssSlab equation.
Not doing so was just a silly mistake.
That involved fixing the tests and fixing bugs.
Kokkos doesn't like std, so use Utils. One use of _ft to prevent promotion to float.
einola
marked this pull request as draft
August 21, 2026 06:53
The default in old nextsim was 30 days, but that seems to be too long here. The reason is probably that in the Lagrangian version, the slab ocean is advected with the ice, so excessively saline waters created in polynyas will be transported away from the coast. I could probably try to find a more nuanced value than one week, but it's good enough for a default.
einola
marked this pull request as ready for review
August 24, 2026 05:41
I don't need an `if (cice[i] > 0)` any more when setting things to zero for too little ice. This is because there is no division by cice[i] now, after I changed the meaning of deltaHi.
Member
Author
|
It now runs for a full year with ok looking results (since we don't have snow cover or precipitation). The only thing to note is that I can get it to work with DG1 only. With DG2, I get a crash in the dynamics. This should be a different issue. |
Thanduriel
approved these changes
Aug 25, 2026
Thanduriel
left a comment
Thanduriel
left a comment
Member
There was a problem hiding this comment.
Works for me. In fact, DG2 looks ok as well. I am still running with a time-step of 5min and 120 BBM sub-steps.
A closer look at the solutions in december:
Member
Author
|
@Thanduriel Thanks for confirming it works. I'll try DG2 again, but I'm confused as to why it didn't work for me. Can you add the config file you used to this PR? |
Member
Build options: |
Member
Author
|
DG2 works for me now as well. Don't know what I did wrong before, though. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix salinity calculation in slab ocean
Fixes #1088
Task List
Change Description
The code wasn't using the salt flux calculated by IOCeanBoundary::mergeFluxes. This may be causing problems in the spring. Not fully tested yet!
Test Description
It doesn't crash immediately, but I need @Thanduriel's help to test it on his setup.
Documentation Impact
None
Other Details
None