Bring in changes from AKT icepack-integration branch #1

darincomeau · 2022-12-09T20:07:07Z

Begins installation of Icepack as an alternative column physics package in MPAS-seaice. The new namelist flag config_column_physics_type chooses between them. Calls using icepack_intfc.F90 routines activate Icepack physics.

This PR cherry-picks in changes introduced in @akturner 's branch https://github.com/akturner/E3SM/tree/akturner/mpas-seaice/icepack-integration.

[BFB] with the current branch (eclare108213/seaice/icepack-integration) with the default config_column_physics_type = 'column_package'.

Add icepack as new submodule Add build of icepack to MPAS-Seaice build system Add extra variables to Registry new to icepack Added mpas_seaice_icepack module as replacement for mpas_seaice_column module. This is initially a straight copy of mpas_seaice_column Replaced all calls to mpas_seaice_column module with calls to mpas_seaice_icepack. Except radiation (as described below) mpas_seaice_icepack still calls the column package Replace column radiation calls from ice_colpkg to icepack_intfc in mpas_seaice_icepack Add initialization of icepack parameters/tracers to mpas_seaice_icepack Add initialization of icepack shortwave data structures to mpas_seaice_icepack Renamed exposed subroutines in mpas_seaice_icepack from _column_ to _icepack_ Replaced few remaining references to ice_colpkg from files other than mpas_seaice_column.F and mpas_seaice_icepack.F with extra subroutines in those files Note: icepack_warnings, icepack_warnings_getall needs to be public Conflicts: .gitmodules components/mpas-seaice/src/icepack components/mpas-seaice/src/shared/mpas_seaice_constants.F

…amelist

darincomeau · 2022-12-09T20:44:38Z

components/mpas-seaice/bld/build-namelist

@@ -620,6 +620,7 @@ add_default($nl, 'config_recover_tracer_means_check');
 # Namelist group: column_package #
 ##################################

+add_default($nl, 'config_column_physics_type');


Here and elsewhere for this namelist option, I'm not sure if under the column_package group is the right place for it, but I kept it consistent with the existing entry in Registry.xml.

… set to be on

darincomeau · 2022-12-09T23:15:34Z

Note this is crashing on init with config_column_physics_type = 'icepack'; we'd like this to run with that option, and still use the column package step_radiation, and then turn that on to the icepack version in a subsequent PR.

eclare108213 · 2022-12-09T23:40:44Z

When icepack is turned on, the current code crashes due to a namelist discrepancy:

CRITICAL ERROR: Use config_shortwave_type='dEdd_snicar_ad' for Snicar in Icepack

This can be fixed without changing MPAS-seaice's namelist inputs by defining a temporary character string to send to icepack:

if (config_use_snicar_ad .and. trim(config_shortwave_type) == 'dEdd') then
tmp_config_shortwave = 'dEdd_snicar_ad'
else
tmp_config_shortwave = trim(config_shortwave_type)
endif
call icepack_init_parameters(& ...
shortwave_in = tmp_config_shortwave, &

The challenge is setting tmp_config_shortwave to the correct character type to match mpas-framework's assumptions. See mpas-framework/src/framework/mpas_pool_routines.F, subroutine mpas_pool_add_config_char for its character declaration.

eclare108213 · 2022-12-09T23:43:47Z

There are temporary ifdefs in a couple of modules, wrapping calls into icepack.
#ifdef icepackON
Since icepackON is not defined, code within these wrappers is not seen by the compiler. These are for testing differences produced by various sections of code (once they are implemented) and will not be kept in the final PR into E3SM.

…car_ad calls

eclare108213 · 2022-12-15T04:42:27Z

This code builds, runs and successfully completes (!) in E3SM ice-only (D) cases with either 'column_package' or 'icepack' for config_column_physics_type, although some configuration options have been changed from E3SM defaults in our tests:

config_use_advection = false
config_use_velocity_solver = false
config_use_column_snow_tracers = false
config_update_ocean_fluxes = false
config_use_effective_snow_density = false
config_use_snow_grain_radius = false
config_use_snicar_ad = false
config_use_snow_liquid_ponds = .false.

For icepack cases, only the initialization and radiation calls are active (but note that snicar_ad is off), all others still use MPAS-seaice column physics. NB The icepack radiation code is not fully implemented here - many changes have been made in icepack since the present code was initially implemented in stand-alone MPAS-seaice. We also need to sort out potential orbital differences. These things can go in a later PR.

Let's merge this PR soon. But before doing so, consider the following:

Find out if any of the above configs can be set back to true, to be closer to standard E3SM configurations. The snow configs must remain false until icepack's snow module is properly implemented.
Double-check that the 'column_package' configuration is still BFB.
Now would be a good time to look at the output, to see how different the results are with just the radiation changes. I guess we can do those runs at any point, based on the hashes. Let's at least check that the 'icepack' output is in the ballpark.

I'll continue working on this tomorrow...

darincomeau · 2022-12-15T16:55:14Z

@eclare108213 we want to revert 85146bf that added in the ifdef's in mpas_seaice_initialize.F, correct?

With that change (removing those ifdef's), with the above namelist changes, I'm getting the model crashing on the first timestep with a thermo Picard convergence error. Have you seen that?

eclare108213 · 2022-12-15T17:09:43Z

My test completed a full simulation using this script:
/home/ac.eclare/SimulationScripts/archive/PPSLRCI/Icepack/D.SMS.Icepack.dcomeau.sh
which pulls code from your fork and also includes some namelist changes. In addition, I had modified some of the code:

#	modified:   src/icepack (modified content)
#	modified:   src/shared/mpas_seaice_icepack.F
#	modified:   src/shared/mpas_seaice_initialize.F
#	modified:   src/shared/mpas_seaice_time_integration.F

Let me look to see how much of that is significant...

The icepack change comments out the rsnow save for history in icepack/columnphysics/icepack_shortwave.F90.
mpas_seaice_icepack.F has the use_snow flag and the addition of an MPAS-pool_get_config call for snicar_ad. I think you have both of those now.
mpas_seaice_initialize.F and mpas_seaice_time_integration.F both commented out ifdefs we had added earlier. Maybe yours still has those in time_integration?

darincomeau · 2022-12-15T17:21:58Z

Maybe yours still has those in time_integration?

Yes I was, missed that one. Running a test now and then I'll revert that commit.

darincomeau · 2022-12-15T18:13:56Z

I now have this running successfully with a slightly smaller number of namelist changes:

config_column_physics_type = 'icepack'
config_use_column_snow_tracers = false
config_use_effective_snow_density = false
config_use_snow_grain_radius = false
config_use_snicar_ad = false
config_use_snow_liquid_ponds = false

eclare108213 · 2022-12-15T18:21:25Z

I will look into the snicar_ad issue. It should run but the initialization steps need to be checked. Also, it might fail on a thermo error if the restart file is not consistent with the changed fluxes. How would I start from a no-ice condition?

darincomeau · 2022-12-15T18:29:52Z

OK this PR is [BFB] with the base branch with default namelist options (so using column package, no icepack).

darincomeau · 2022-12-15T18:30:58Z

How would I start from a no-ice condition?

Namelist changes:

 config_initial_ice_area = 0.0
 config_initial_ice_volume = 0.0

darincomeau · 2022-12-15T18:41:18Z

Note for the successful runs, the following change was made locally (not in this PR) in the icepack submodule:

diff --git a/columnphysics/icepack_shortwave.F90 b/columnphysics/icepack_shortwave.F90
index bb05dec..52bbb05 100644
--- a/columnphysics/icepack_shortwave.F90
+++ b/columnphysics/icepack_shortwave.F90
@@ -1198,11 +1198,11 @@ subroutine run_dEdd(dt,                  &
             if(present(fswthrun_idr)) fswthrun_idr(n) = l_fswthru_idr
             if(present(fswthrun_idf)) fswthrun_idf(n) = l_fswthru_idf

-            if (present(rsnow) .and. .not. snwgrain) then
-               do k = 1,nslyr
-                  rsnow(k,n) = rsnwn(k) ! for history
-               enddo
-            endif
+!            if (present(rsnow) .and. .not. snwgrain) then
+!               do k = 1,nslyr
+!                  rsnow(k,n) = rsnwn(k) ! for history
+!               enddo
+!            endif

eclare108213

Testing is ongoing. The current code compiles and runs with some manual changes as noted in the comments. With that configuration, results are [BFB] when only changing `config_column_physics_type = column_package' to 'icepack'. Since the radiation calculations are using icepack code, this is a little surprising and so we are continuing to look into it.

akturner and others added 9 commits December 9, 2022 11:44

Changes needed for BFB icepack/column pckg shortwave

0df4339

Add namelist option to run with either icepack or column package

afe58d2

Update icepack interface for newer icepack commit

73f41c5

Changes for e3sm build

80bb443

Minor changes for existing E3SM consistency

5b1c863

Removing duplicate use calls

4ddee4a

Adding new config_column_physics_type namelist option to E3SM build n…

2b5a9d7

…amelist

Additional changes for E3SM build namelist

bfab10f

eclare108213 self-requested a review December 9, 2022 20:25

eclare108213 self-assigned this Dec 9, 2022

Correcting erroneous 'use' in new namelist option

4fb0fe0

darincomeau commented Dec 9, 2022

View reviewed changes

darincomeau added 2 commits December 9, 2022 15:57

Correcting new namelist option type

d1de208

Adding ifdefs to turn off icepack_step_radiation even when icepack is…

85146bf

… set to be on

darincomeau added 6 commits December 13, 2022 14:37

Changing 'gregorian_noleap' to 'noleap'

e62effe

Changes to config_shortwave_type for icepack

a6a5afe

Changing tmp config shortwave_type to pointer

068b476

Adding local use_snow logical

c36c445

Adding missing config_use_snicar_ad pool call

2e219f2

Fixing location of use_snow local logical, removing unused config_sni…

807b992

…car_ad calls

Reverting addition of ifdef statements

85e3174

eclare108213 approved these changes Dec 16, 2022

View reviewed changes

eclare108213 merged commit 2ad4b45 into eclare108213:eclare108213/seaice/icepack-integration Dec 16, 2022

njeffery mentioned this pull request Sep 30, 2023

Physics cleanup #37

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bring in changes from AKT icepack-integration branch #1

Bring in changes from AKT icepack-integration branch #1

darincomeau commented Dec 9, 2022 •

edited by eclare108213

Loading

darincomeau Dec 9, 2022

darincomeau commented Dec 9, 2022

eclare108213 commented Dec 9, 2022

eclare108213 commented Dec 9, 2022

eclare108213 commented Dec 15, 2022

darincomeau commented Dec 15, 2022

eclare108213 commented Dec 15, 2022

darincomeau commented Dec 15, 2022

darincomeau commented Dec 15, 2022

eclare108213 commented Dec 15, 2022

darincomeau commented Dec 15, 2022

darincomeau commented Dec 15, 2022

darincomeau commented Dec 15, 2022

eclare108213 left a comment

Bring in changes from AKT icepack-integration branch #1

Bring in changes from AKT icepack-integration branch #1

Conversation

darincomeau commented Dec 9, 2022 • edited by eclare108213 Loading

darincomeau Dec 9, 2022

Choose a reason for hiding this comment

darincomeau commented Dec 9, 2022

eclare108213 commented Dec 9, 2022

eclare108213 commented Dec 9, 2022

eclare108213 commented Dec 15, 2022

darincomeau commented Dec 15, 2022

eclare108213 commented Dec 15, 2022

darincomeau commented Dec 15, 2022

darincomeau commented Dec 15, 2022

eclare108213 commented Dec 15, 2022

darincomeau commented Dec 15, 2022

darincomeau commented Dec 15, 2022

darincomeau commented Dec 15, 2022

eclare108213 left a comment

Choose a reason for hiding this comment

darincomeau commented Dec 9, 2022 •

edited by eclare108213

Loading