Skip to content

fix metric terms in sea-ice stress divergence#976

Merged
jm-c merged 15 commits intoMITgcm:masterfrom
mjlosch:seaice_fix_metric_terms
May 4, 2026
Merged

fix metric terms in sea-ice stress divergence#976
jm-c merged 15 commits intoMITgcm:masterfrom
mjlosch:seaice_fix_metric_terms

Conversation

@mjlosch
Copy link
Copy Markdown
Member

@mjlosch mjlosch commented Feb 27, 2026

What changes does this PR introduce?

Bug fix

What is the current behaviour?

When I implemented the stress divergence of sea ice, I thought that treating the individual components of the stress tensor like scalars would suffice to take care of all metric terms (see documentation and Losch et al 2010), but little did I know about tensor calculus: you also need to differentiate the unit vectors and this leads to terms that cannot be expressed in flux form. These terms are missing.

What is the new behaviour?

Two extra metric terms of the stress divergence each are added to the u- and v-equation:

u: k_{2}\sigma_{12} - k_{1}\sigma_{22}
v: k_{1}\sigma_{12} - k_{2}\sigma_{11}

For all solvers these terms are added to the RHS of the equation. For EVP and the implicit Krylov/JNFK solvers this is simple, but for the LSR solver this means that actually some part of the u/v-terms are not treated implicitly. This may seem inconsistent (as other metric terms are treating implicitly), but it is sooo much easier this way.

Does this PR introduce a breaking change?

All results on curvilinear grids change (a little). Fortunately most sea-ice simulations are carried out on cubed sphere or llc-grid where the Arctic is on some sort of 'near-cartesian' grid and the metric terms are small. As an example I attach results from a test on a 36km grid (carved out from a cubed sphere, basically coarse grained from the 18km grid that was used in Nguyen et al 2011, etc).

I ran 3 12-year-simulations: "default" (with the extra metric terms missing), "all metric terms", "all k1/2=0"; the latter means that the metric terms in the strain rates and the extra metric terms in the stress divergence are set to zero, but the metric terms that are covered by the finite volume discretisation are retained (I cannot change that without large incisions, which I do not want to make). From the timeseries of standard deviation of differences it's clear that the differences do not increase much after 2-3 years, so that it is probably OK to compare April 2012 (the 12th year).

In general the differences I see are small (order 1-10 centimetres, although some larger values appear every now and then). A little surprising: the order of magnitude of differences does not really change between the different comparisons, I had expected that the effect of the metric terms in the strain rates would be larger. My interpretation is that in these runs there are not really any systematic differences, just different realisations of "noise" that lead to a similar orders of magnitude of differences.

cmp_siheff_timeseries cmp_siheff

Other information:

We need to decide, what the defaults should be. There's a new runtime flag SEAICEselectMetricTerms, that is currently 2 (use all metric terms), which can be set to 1 to recover old results (ignore current bug fix), and 0 to also set k_1/2=0 for the computation of the metric terms in the strain rates.

Suggested addition to tag-index

o pkg/seaice: add missing metric terms to stress divergence (bug fix)

  • changes all results on curvilinear grids (update reference results)
  • SEAICEselectMetricTerms = 1 turns off new terms to recover old results (tested in lab_sea)

- add metric factors for at U- and V-points also for C-grid in order
  to be able to use them for the extra metric terms to come
- simplify seaice_init_fixed.F a little and avoid repetitive code
- add new runtime flag SEAICEuseExtraMetricTerms = T (default)
- changes results of all experiments with a curvilinear grid
@mjlosch
Copy link
Copy Markdown
Member Author

mjlosch commented Mar 2, 2026

I can easily recover old (and wrong) results by setting SEAICEuseExtraMetricTerms = .FALSE., in

verification/global_ocean.cs32x15/input.icedyn/data.seaice
verification/global_ocean.cs32x15/input.seaice/data.seaice
verification/lab_sea/input.hb87/data.seaice
verification/lab_sea/input.salt_plume/data.seaice
verification/lab_sea/input/data.seaice
verification/seaice_itd/input.thermo/data.seaice
verification/seaice_obcs/input.regDenom/data.seaice
verification/seaice_obcs/input/data.seaice

and for the AD-expeirments

verification/global_ocean.cs32x15/input_ad.seaice/data.seaice
verification/global_ocean.cs32x15/input_ad.seaice_dynmix/data.seaice
verification/lab_sea/input_ad/data.seaice
verification/lab_sea/input_tap.noecco/data.seaice

mjlosch added 5 commits March 4, 2026 17:17
- bug fix
- make notation a little more consistent
fix description of tensor algebra and indices, hopefully consistent now
- add subsection title "zero-layer thermodynamics"
- describe longwave radiation balance more accurately
@jm-c jm-c self-requested a review March 25, 2026 15:23
@jrscott jrscott self-requested a review March 25, 2026 15:24
@jm-c
Copy link
Copy Markdown
Member

jm-c commented Apr 13, 2026

@mjlosch I have downloaded your branch, and merged updated master branch in it (since I need to updated few ref. ouput) but I am unable to push the changes to your branch (permission denied). Could you check that you allow developpers to make changes ?

@mjlosch
Copy link
Copy Markdown
Member Author

mjlosch commented Apr 14, 2026

@mjlosch I have downloaded your branch, and merged updated master branch in it (since I need to updated few ref. ouput) but I am unable to push the changes to your branch (permission denied). Could you check that you allow developpers to make changes ?

@jm-c, sorry, I don't understand why that wasn't ticked. Please try again.

Copy link
Copy Markdown
Member

@jm-c jm-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a quick look at code changes, looks good to me, and left a comment related to half-retired SEAICEuseMetricTerms.
Later on, will generate new reference output for the affected experiments after checking that these exp. results are unchanged if I set SEAICEselectMetricTerms=1. There are also few reference output to update in verification_other so will need a new PR there (I will work on that soon).

Comment thread pkg/seaice/SEAICE_GRID.h
#if ( defined SEAICE_CGRID || defined SEAICE_BGRID_DYNAMICS )
C k1/2AtC/U/V :: coefficients at C, U, and V points
C for metric terms in U/V ice equations.
COMMON /ARRAYMETRIC/ k1AtC, k2AtC, k1AtU, k1AtV, k2AtU, k2AtV
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A very minor comment: could change the ordering of these coeff in common block and declaration below, either to:

k1AtC, k2AtC, k1AtU, k2AtU, k1AtV, k2AtV

or:

k1AtC, k1AtU, k1AtV, k2AtC, k2AtU, k2AtV

Comment thread pkg/seaice/seaice_readparms.F Outdated
SEAICEuseTD = .FALSE.
SEAICEusePL = .FALSE.
SEAICEuseMetricTerms = .TRUE.
SEAICEuseMetricTerms = .FALSE.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment on how we handle this half retired run-time parameter "SEAICEuseMetricTerms":
With master branch, the default was True, and when user set it to False in data.seaice the metric-terms were removed (corresponding to SEAICEselectMetricTerms=0 with this branch). If we change the new default to False, the same setting in data.seaice (i.e., set to False) will result in having the full set of metric terms on (default value of SEAICEselectMetricTerms) according to the code below. So, to me, it does not look like a bacward compatibility solution.

Copy link
Copy Markdown
Member Author

@mjlosch mjlosch Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new default is SEAICEselectMetricTerms = 2 (ll916, implying SEAICEuseMetricTerms=T independent of how this was set). If the user decides to set SEAICEselectMetricTerms = 0, this would be overridden by a default SEAICEuseMetricTerms=T, so I set the default to F. I guess we need a solution where (in data.seaice):

  • SEAICEuseMetricTerms=.FALSE., leads to SEAICEselectMetricTerms = 0
  • SEAICEuseMetricTerms=.TRUE., (default setting), leads to SEAICEselectMetricTerms = 2

Do we need to catch more combinations, some which make little sense like SEAICEuseMetricTerms=.FALSE. + SEAICEselectMetricTerms = 1/2, or SEAICEuseMetricTerms=.TRUE. + SEAICEselectMetricTerms = 0?
If not, this would probably do, wound't it (with old default SEAICEuseMetricTerms=T)?

      SEAICEuseMetricTerms = .TRUE.
      SEAICEselectMetricTerms = UNSET_I
C
[...] read namelist
C
      IF ( usingCartesianGrid .OR. .NOT.SEAICEuseMetricTerms )
     &     SEAICEselectMetricTerms = 0
      IF ( SEAICEselectMetricTerms .EQ. UNSET_I ) 
     &     SEAICEselectMetricTerms = 2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the general idea is that SEAICEuseMetricTerms is partially retired in favor of setting SEAICEselectMetricTerms. So if the 2 are set in data.seaice to incompatible values, should STOP with error message.
And the resetting of SEAICEselectMetricTerms to zero for Cartesian-Grid could be applied after the other logic and settings.
I could try and push something there if you prefer and you will be able to adjust/change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I am usually not very good with combining logic, I'd prefer if you'd give it a shot.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just pushed something, please take a look to see if this makes sense (and feel free to make changes).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good me!

@jm-c
Copy link
Copy Markdown
Member

jm-c commented Apr 15, 2026

I have an other comment: When using a Cartesian-Grid, we could switch/force SEAICEselectMetricTerms to be zero. I think it's what we do for the dynamics selectMetricTerms. And this will save a little computation.

jm-c added 2 commits April 17, 2026 01:19
from experiments using seaice dynamics on Lat-Lon or Curvilinear Grid
Copy link
Copy Markdown
Member

@jm-c jm-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jm-c
Copy link
Copy Markdown
Member

jm-c commented Apr 23, 2026

I've just made a new PR in verification_other ( MITgcm/verification_other#80 ) to update things there once this PR is merged. So, from my side, this PR is ready to be merged.

@jrscott
Copy link
Copy Markdown
Member

jrscott commented Apr 30, 2026

LGTM
back to you @jm-c

@jm-c
Copy link
Copy Markdown
Member

jm-c commented May 2, 2026

@mjlosch Do you agree that this PR is good to be merged ? If yes, will so that soon after.

@mjlosch
Copy link
Copy Markdown
Member Author

mjlosch commented May 4, 2026

I have nothing to add. Let's merge this.

@jm-c jm-c merged commit 3f0f10f into MITgcm:master May 4, 2026
19 checks passed
@mjlosch mjlosch deleted the seaice_fix_metric_terms branch May 4, 2026 15:23
Hoda-Hashemi pushed a commit to Hoda-Hashemi/MITgcm that referenced this pull request May 8, 2026
* move masks and metric factors from SEAICE.h new SEAICE_GRID.h

- add metric factors for at U- and V-points also for C-grid in order
  to be able to use them for the extra metric terms to come
- simplify seaice_init_fixed.F a little and avoid repetitive code

* add extra metric terms for all solvers and to documenation

- add new runtime flag SEAICEuseExtraMetricTerms = T (default)
- changes results of all experiments with a curvilinear grid

* improve documentation, better description of stress divergence

- bug fix
- make notation a little more consistent

* replace logical flags with integer flag SEAICEselectMetricTerms

* the documentation is getting better and better

fix description of tensor algebra and indices, hopefully consistent now

* unrelated improvement of documentation (thermodynamics)

- add subsection title "zero-layer thermodynamics"
- describe longwave radiation balance more accurately

* a little reorganisation

* report value of SEAICE_useMultDimSnow

- sneak in fix that is unrelated to this PR

* minor: upper/lower case consistency

* update SEAICEuseMetricTerms setting

* update affected reference output

from experiments using seaice dynamics on Lat-Lon or Curvilinear Grid

* minor doc edit, thermo section

* document seaice-metric terms update

---------

Co-authored-by: Jean-Michel Campin <jmc@mit.edu>
Co-authored-by: JEFFERY SCOTT <jscott@dhcp-10-29-193-64.dyn.mit.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants