Ice ridging from Icepack #147
Conversation
- These modifications enable ridging code for SIS2 C-grid
configurations (not yet implemented for B-grid) using APIs
provided by the Icepack module from the CICE consortium
(https://github.com/CICE-Consortium/Icepack)
- These APIs are reproduced in the config_src/external/Icepack
directory in order to remove SIS2 dependency on Icepack by default.
In order to activate Icepack, remove the config_src/external/Icepack
diretory from your compile list, compile a shared Icepack library
and link to SIS2.
- Sea-ice ridging rate diagnostics (rdg_rate) report the fractional
rate of sea-ice coverage decrease (s-1) due to the ridging scheme.
- WARNING:Sea-ice enthalpy diagnostics indicate a decrease in numerical closure
from roundoff (10-17) without ridging enabled to values greater
than roundoff (10-7) in initial testing. Mass and salinity,
however, are consistent to roundoff. The issue should be addressed
before using this option to generate scientific results.
- The default Icepack ridging options are enabled. Additional options
can be enabled easily using the SIS_parameter file and calls to
icepack_init_parameters
- This appears to be needed to avoid propagation of
negative ice free categories. Residual ice excess values
of order 10-5 are being renormalized.
|
@Hallberg-NOAA @kshedstrom I forgot to add a couple interfaces to Icepack in the config_src/external directory. Updating dev/esmg with ESMG#4 will resolve that issue. |
additional Icepack interfaces
|
@Hallberg-NOAA @marshallward . I have updated dev/esmg, so this PR should compile w/o importing Icepack after updating your local branch. |
| public :: icepack_init_tracer_sizes | ||
| public :: icepack_query_tracer_sizes | ||
|
|
||
| real(kind=dbl_kind), parameter, public :: n_iso=0,n_aero=0 |
There was a problem hiding this comment.
This line (18) is causing the gnu compiler to fail, because that compiler does not permit reals to be used when declaring the size of memory. However it does successfully compile if this line is changed to integer (kind=int_kind), parameter, public :: n_iso=0, n_aero=0.
There was a problem hiding this comment.
With one minor change to the code on line 18 of icepack_tracers.F90, SIS2 is now compiling with this PR.
However, this code violates the MOM6/SIS2 coding standards in a number of ways:
- There are several 'module use' instructions that do not include the 'only' attribute.
- There are module use statements for variables from other modules. SIS2 permits the module use of interfaces and types, but not of variables themselves, as this inhibits detection of where variables are actually used and precludes the use of ensembles where these variables might vary.
- There are a large number of module variables introduced. This is strongly discouraged in MOM6 and SIS2 code, with the exceptions of timers or integer parameters used to encode options.
- A large number of subroutine arguments are not described in comments, and none of them use Doxygen.
- The units of a number of real variables are not documented.
- There are 102 instances of lines with trailing white-space
Given that this is essentially the importation of external code from Icepack, some of these are hard to address, whereas others are trivial. Once the code is compiling and running properly with all 3 of our usual compilers (intel, gnu and pgi), I think that we can accept this PR, but we should immediately address these issues or formally log them as SIS2 issues.
Changes parameter n_iso and n_aero type to integer
|
Something changed in the last updates. Before, ice thickness looked like this in ncview: |
|
I have a question about the build procedure. If I link to the full Icepack, should I also be linking to the codes in SIS2/config_src/external/Icepack_interfaces? I clearly am linking to them currently, as well as linking to Icepack. The config_src routines trump those in Icepack if the names match. Putting the guts of icepack_init_tracer_sizes back, copied over from Icepack, gets me running again. |
|
You should never include both the config-src/external/Icepack_interfaces code and the Icepack code itself. The former exist only to allow us to compile without needing to have the Icepack package. I suppose that we could consider putting some limited subset of the Icepack capabilities into the Icepack-interfaces code, but I am not sure that I understand the reason why we would do so. A strategy that I would prefer would be to issue a fatal error or a warning message if we tried to call (or maybe use) the Icepack_interfaces code, since that would indicate that something is inconsistent between what the user probably intends and the way the model has been compiled. |
| @@ -141,9 +141,30 @@ subroutine icepack_init_tracer_sizes(& | |||
| nbtrcr_in , & ! number of bio tracers in use | |||
| nbtrcr_sw_in ! number of shortwave bio tracers in use | |||
|
|
|||
| if (present(ncat_in) ) ncat = ncat_in | |||
There was a problem hiding this comment.
This block of code is setting a series of variables that do not exist, so the code no longer compiles.
I believe that when the changes in commit 2acd393 were merged into dev/esmg, they inadvertently undid the proper changes from commit 16e1b4b. If this block of code from lines 144 - 164 were deleted, this code would work again.
- I should not be linking to both this and the full Icepack.
|
I can't honestly test this right now as chinook is acting up. Hopefully I can beat the Makefile into shape. |
|
I have run the full suite of tests that use SIS2, and this PR is now reproducing all of the answers for the existing test cases. Moreover because this new code is enabled by compiling with Icepack and reusing runtime parameters that were already present in the SIS2 code, there are no changes to the output files in the MOM6-examples test suite. However, it should be noted that the new code is not yet being exercised, so for this I am trusting that it is correct based on the tests on the ESMG branch. |
This code requires linking to the Icepack routines.