sensible+latent heatfluxes using linear bulk formula #633

mhrib · 2021-09-14T20:05:06Z

PR checklist

By introducing the "simpler" linear bulk formula for sensible+latent heat fluxes, but preserve the temperature/humidity formulation for stress, the DMI-HYCOM-CICE (ERA5) code got more stable for long runs (several of years) without significant changes in the overall sea-ice cover/thickness. Without these modifications, the "Flux Error" (ferr) - rarely - became very high in special circumstances with low wind speeds and Tair>Tocn. In these situations the Jordan et al (1999) formulation might have it's limitations. These high Flux Errors seems to dissapear with the simpler formulations for sensible and latent heatfluxes for our setup at least.

The results are not BFB with the new setting turned on. However it is implemented turned off, which results in BFB results compared to the unmodified code.

NOTE:

Both ICEPACK and CICE have to be updated, as the "atmbndy" namelist flag goes into both codes with default renamed to 'similarity'
Icepack PR: sensible+latent heatfluxes using linear bulk formula Icepack#371

…linear bulk formula

eclare108213

I don't see any major problems but have a couple of picky suggestions. Please remove the word "traditional" everywhere and use "linear bulk formula" for the PR title. Also, the heatflux_linear flag does't apply to all heat fluxes, so let's find a more specific name for it. I usually refer to sensible+latent as the turbulent heat fluxes. Could use turbheat_linear or turbheatflux_linear or something like that?

eclare108213 · 2021-09-15T00:16:03Z

Actually, looking at the Icepack PR, I'm thinking that the namelist flag should instead be an additional option for atmbndy, e.g. instead of 'default' and 'constant, use 'similarity' (referring to Monin-Obukhov similarity theory), 'mixed' and 'constant'.

apcraig · 2021-09-15T07:57:55Z

If it makes sense to implement the new feature as a new option of an existing namelist and icepack argument, that would be good.

mhrib · 2021-09-15T10:22:19Z

Good point using an existing namelist argument. I'll look at it using 'atmbndy' namelist parameter instead as suggested by @eclare108213 . Also makes the implementation much more simple.

mhrib · 2021-09-15T15:16:26Z

Rewritten using 'atmbndy' namelist variable as suggested.
Note, that 'ice_in' file has changed default setting atmbndy='similarity' (identical to 'default' in previous version)

mhrib · 2021-09-15T21:58:51Z

Update code using 'atmnbdy' which now allows three options: 'similarity' (default), 'constant', or 'mixed'
Also update documentation section 3.4.3.10.forcing_nml and section 5 (index of variables) with the above info

eclare108213

This is good. Icepack will need to be updated here after that PR is merged.

apcraig · 2021-09-16T05:21:23Z

I will add one other thing. I think we should continue to support "default", even if it's not documented, for backwards compatibility. On the CICE side, if "default" is set in namelist, on input it should be reset to "similarity" and a brief message written. The same can be done in the Icepack driver. In Icepack, if "default" is passed in, it should treat it the same as "similarity" either by renaming it as it's passed in or by making "default" and "similarity" appear together in all the logic. Actually Icepack probably does this now because....

One other thing that seems to be the case in Icepack, there is no check anywhere that the atmbndy value is reasonable. The only place it's used is in icepack_atmo.F90. The logic there allows any value to be valid. Is that something we can/should improve? That could be either by checking atmbndy value on input to icepack_parameters (which we don't typically do) or by fixing the if/else logic explicitly (in icepack_atmo) adding an abort if the atmbndy is not valid.

mhrib · 2021-09-16T13:13:48Z

Updated:

Update “Atmosphere” section 2.1.1 with almost the suggested text. (in icepack, not here)
Abort if 'atmbndy' setting is unknown (done via. abort in icepack)
For backward compability: 'atmbndy'=default is renamed to similarity. And a WARNING message is given in that case to the log.

phil-blain · 2021-09-16T13:47:47Z

cicecore/cicedynB/general/ice_init.F90

+            write(nu_diag,*) subname//' WARNING: atmbndy = default'
+            write(nu_diag,*) subname//' WARNING:   For backward compability, set atmbndy = similarity'


Small wording suggestion:

Suggested change

write(nu_diag,*) subname//' WARNING: atmbndy = default'

write(nu_diag,*) subname//' WARNING: For backward compability, set atmbndy = similarity'

write(nu_diag,*) subname//' WARNING: "atmbndy = default" is deprecated, use "atmbndy = similarity"'

write(nu_diag,*) subname//' WARNING: setting atmbndy = similarity'

eclare108213

I have some picky requests re wording in the comments, nothing significant.

eclare108213 · 2021-09-22T18:08:48Z

cicecore/cicedynB/general/ice_init.F90

-         if (trim(atmbndy) == 'default') then
-            tmpstr2 = ' : stability-based boundary layer'
+         if (trim(atmbndy) == 'constant') then
+            tmpstr2 = ' : boundary layer uses bulk transfer coefficients'


I would change 'bulk' to 'constant' in this line. If I remember correctly, the overall parameterization we use is considered a 'bulk' formulation, and we're just choosing whether the coefficients are constant or not. Anyone please correct me if that's wrong. (And yes I used 'bulk' in the original comment!)

Changed / rewritten

eclare108213 · 2021-09-22T18:12:32Z

cicecore/cicedynB/general/ice_init.F90

+            if (trim(atmbndy) == 'similarity') then
+               tmpstr2 = ' : stability-based boundary layer'
+            else
+               tmpstr2 = ' : stability-based boundary layer, but constant for sensible+latent heatfluxes'


How about
' : stability-based wind stress, constant-coefficient sensible+latent heat fluxes'

Changed / rewritten

eclare108213 · 2021-09-22T18:13:33Z

doc/source/user_guide/ug_case_settings.rst

+   "``atmbndy``", "string", "bulk transfer coefficients", "``similarity``"
+   "", "``similarity``", "stability-based boundary layer", ""
+   "", "``constant``", "constant-based boundary layer", ""
+   "", "``mixed``", "stability-based, but constant for sensible+latent heatfluxes", ""


Likewise,
'stability-based wind stress, constant-coefficient sensible+latent heat fluxes'

Changed / rewritten

eclare108213

Getting close

eclare108213 · 2021-09-23T13:13:23Z

cicecore/cicedynB/general/ice_init.F90

@@ -1216,8 +1216,8 @@ subroutine input_data

      if (trim(atmbndy) == 'default') then
         if (my_task == master_task) then
-            write(nu_diag,*) subname//' WARNING: atmbndy = default'
-            write(nu_diag,*) subname//' WARNING:   For backward compability, set atmbndy = similarity'
+            write(nu_diag,*) subname//' WARNING: "atmbndy = default" is decrecated'


spelling error: deprecated
Are the double quotes needed in these write statements?

No, double quotes not necessary and removed again. Was introduced as a suggested update, which I may have interpreted incorrectly.
Also update my spelling error.

eclare108213 · 2021-09-23T20:44:41Z

I've just merged CICE-Consortium/Icepack#371, so now I think you should update Icepack here and then we'll merge this PR.

apcraig

Do we need to add a new test case or two? Otherwise, looks good to me.

mhrib · 2021-09-24T17:11:08Z

@apcraig I don't think it is necessary with new testcase, as the default=similarity is exactly the same as previous.
However if necessary, I think there should be a new testcase for both atmbndy = constant (same as before) AND atmbndy = mixed (new).

apcraig · 2021-09-24T17:15:50Z

Can you add those @mhrib?

mhrib · 2021-09-25T07:05:27Z

@apcraig I am a bit uncertain what to test. All three settings of atmbndy will give different results, but that is on purpose...

apcraig · 2021-09-25T17:20:59Z

@mhrib, I think you should just add a couple new restart tests with the other atmbndy settings. You should add a set_nml.atmbndyconst and set_nml.atmbndymixed in configuration/scripts/options/. Then you should add two new tests to the base_suite (configurations/scripts/tests/base_suite.ts) that would simply be something like

restart        gx3     12x2        atmbndyconst
restart        gx3      8x2        atmbndymixed

Then just make sure those tests pass. I think it would be fine to just run each test manually to confirm.

The other way to do this is to modify a couple of "alt" tests to add the constant and mixed options to those. That would be more efficient and I would support that assuming it's clear which alt tests would be reasonable to modify. In that case, you'd just modify the set_nml.alt* files and not add the new set_nml files. Either way seems reasonable. Thanks.

mhrib · 2021-09-27T10:13:43Z

@apcraig Thanks for explanation for creating relevant tests.
Now adding test-cases following your first approach adding 2x restart tests using atmbndy={constant,mixed} to the base_suite.

All test - including the 2 new ones - passes with "SUCCESSFULLY" on the DMI computer (freya), except for three tests. These three unsuccessfully runs all tries to allocate more OpenMP processors than cores/node, which is obviously not valid. Ie. in total 118 good and 3 not started tests from the base_suite tests.

So now I think the PR is ready to be merged...

apcraig

I have a slight preference for not having special characters in the set_nml filenames (set_nml.atmbndyconst vs set_nml.atmbndy_const) as I worry about parsing a little and for consistency. But I think this works. Will approve.

mhrib · 2021-09-27T20:21:35Z

@apcraig I have changed to "non-underscore".
Not sure I fully understand the problem wrt. underscore "_" in text unless we're talking LaTeX interpretter, text writing in plotting routines etc.. Nevertheless, the functionality remains unchanged.

eclare108213 · 2021-09-27T21:20:26Z

@mhrib Thanks for your patience and diligence, working through all the various requests! This looks good - thanks also for adding this flexibility to the code.

mhrib · 2021-09-28T08:30:14Z

@eclare108213 I really appreciate the various requests.
It has ended up in a much simpler solution without introducing an extra namelist parameter as original purposed. Thereby much easier to understand especially in combination with the requested documentation. We at DMI appreciate the inclusion of this new feature, which makes future updates of CICE easier for us. So a win-win for all :-)

'heatflux_linear' flag: sensible+latent heatfluxes using traditional …

db07a38

…linear bulk formula

mhrib mentioned this pull request Sep 14, 2021

sensible+latent heatfluxes using linear bulk formula CICE-Consortium/Icepack#371

Merged

16 tasks

eclare108213 requested changes Sep 15, 2021

View reviewed changes

Add option atmbndy='mixed' boundary layer condition

ce7b41d

mhrib changed the title ~~sensible+latent heatfluxes using traditional bulk formula~~ sensible+latent heatfluxes using linear bulk formula Sep 15, 2021

New options for 'atmbndy'

416bd74

eclare108213 approved these changes Sep 15, 2021

View reviewed changes

For backward compability, rename "atmbndy" = default to similarity

316538c

phil-blain reviewed Sep 16, 2021

View reviewed changes

eclare108213 requested changes Sep 22, 2021

View reviewed changes

mhrib added 2 commits September 23, 2021 07:07

Change wording for text related to atmbndy

bc292e5

Change wording for text related to atmbndy

779509a

mhrib requested a review from eclare108213 September 23, 2021 09:31

Change wording for text related to atmbndy

b759d49

eclare108213 requested changes Sep 23, 2021

View reviewed changes

Spelling error etc.

d3dd99e

eclare108213 approved these changes Sep 23, 2021

View reviewed changes

Update Icepack with required atmbndy changes

3954d4b

eclare108213 requested a review from apcraig September 24, 2021 12:13

apcraig approved these changes Sep 24, 2021

View reviewed changes

mhrib added 2 commits September 27, 2021 07:25

Added test for atmbndy={constant,mixed}

fa92c7f

Add test for atmbndy={constant,mixed}

dd18f9b

apcraig approved these changes Sep 27, 2021

View reviewed changes

Test for atmbndy={constant,mixed}

9d2aacb

clean up. Renamed without underscores

290744b

eclare108213 merged commit 6c4160e into CICE-Consortium:master Sep 27, 2021

mhrib deleted the CICE branch October 7, 2021 12:50

TillRasmussen mentioned this pull request Oct 16, 2021

Residual ice #645

Open

DeniseWorthen mentioned this pull request Nov 30, 2021

Update CICE to Consortium Main NOAA-EMC/CICE#44

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sensible+latent heatfluxes using linear bulk formula #633

sensible+latent heatfluxes using linear bulk formula #633

mhrib commented Sep 14, 2021 •

edited

Loading

eclare108213 left a comment

eclare108213 commented Sep 15, 2021

apcraig commented Sep 15, 2021

mhrib commented Sep 15, 2021 •

edited

Loading

mhrib commented Sep 15, 2021 •

edited

Loading

mhrib commented Sep 15, 2021

eclare108213 left a comment

apcraig commented Sep 16, 2021 •

edited

Loading

mhrib commented Sep 16, 2021

phil-blain Sep 16, 2021

mhrib Sep 23, 2021

eclare108213 left a comment

eclare108213 Sep 22, 2021

mhrib Sep 23, 2021

eclare108213 Sep 22, 2021

mhrib Sep 23, 2021

eclare108213 Sep 22, 2021

mhrib Sep 23, 2021

eclare108213 left a comment

eclare108213 Sep 23, 2021

mhrib Sep 23, 2021 •

edited

Loading

eclare108213 Sep 23, 2021

eclare108213 commented Sep 23, 2021

apcraig left a comment

mhrib commented Sep 24, 2021

apcraig commented Sep 24, 2021

mhrib commented Sep 25, 2021

apcraig commented Sep 25, 2021

mhrib commented Sep 27, 2021 •

edited

Loading

apcraig left a comment

mhrib commented Sep 27, 2021

eclare108213 commented Sep 27, 2021

mhrib commented Sep 28, 2021

		write(nu_diag,*) subname//' WARNING: atmbndy = default'
		write(nu_diag,*) subname//' WARNING: For backward compability, set atmbndy = similarity'

sensible+latent heatfluxes using linear bulk formula #633

sensible+latent heatfluxes using linear bulk formula #633

Conversation

mhrib commented Sep 14, 2021 • edited Loading

PR checklist

eclare108213 left a comment

Choose a reason for hiding this comment

eclare108213 commented Sep 15, 2021

apcraig commented Sep 15, 2021

mhrib commented Sep 15, 2021 • edited Loading

mhrib commented Sep 15, 2021 • edited Loading

mhrib commented Sep 15, 2021

eclare108213 left a comment

Choose a reason for hiding this comment

apcraig commented Sep 16, 2021 • edited Loading

mhrib commented Sep 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eclare108213 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eclare108213 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhrib Sep 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eclare108213 commented Sep 23, 2021

apcraig left a comment

Choose a reason for hiding this comment

mhrib commented Sep 24, 2021

apcraig commented Sep 24, 2021

mhrib commented Sep 25, 2021

apcraig commented Sep 25, 2021

mhrib commented Sep 27, 2021 • edited Loading

apcraig left a comment

Choose a reason for hiding this comment

mhrib commented Sep 27, 2021

eclare108213 commented Sep 27, 2021

mhrib commented Sep 28, 2021

mhrib commented Sep 14, 2021 •

edited

Loading

mhrib commented Sep 15, 2021 •

edited

Loading

mhrib commented Sep 15, 2021 •

edited

Loading

apcraig commented Sep 16, 2021 •

edited

Loading

mhrib Sep 23, 2021 •

edited

Loading

mhrib commented Sep 27, 2021 •

edited

Loading