Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Icepack #750

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft

Icepack #750

wants to merge 23 commits into from

Conversation

mjs2369
Copy link
Contributor

@mjs2369 mjs2369 commented Oct 3, 2024

Description:

Adds the DART interface to Icepack, the CICE Single Column Model

This new model code is due to the work done by @mollymwieringa and @criedel40

This PR is on draft because there are a few things that still need worked out. Namely, updating the model_interpolate lon_lat_interpolate to use state_structure_mod routines instead and editing the commit history to give Molly and Chris authorship on the initial commit

This PR will not include any scripting to assist our users with running the assimilations, these will be added in a separate PR. The same goes for new observation converters for sea ice observations

Here is the original specification

Fixes issue

#475

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Documentation changes needed?

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.

Tests

Compiled and ran the Icepack code with full debugging flags with ifort and gfortran
Tested to be bitwise identical with original icepack code on the branch cice-scm

Checklist for merging

  • Updated changelog entry
  • Documentation updated
  • Update conf.py

Checklist for release

  • Merge into main
  • Create release from the main branch with appropriate tag
  • Delete feature-branch

Testing Datasets

  • Dataset needed for testing available upon request
  • Dataset download instructions included
  • No dataset needed

Test cases can be found here:
/glade/derecho/scratch/masmith/DART_latest/DART/models/icepack/tests

Information on them can be found in the README file in their respective directories
Note that for these tests, the assimilation is only performed over a single time window and therefore the model is never advanced.

hkershaw-brown and others added 18 commits August 26, 2024 16:30
cice single colum model work from:
Molly Wieringa
Chris Riedel
Cecilia Bitz

This reverts commit 0932d47.
Sea Ice Single Column Model
…register_module, adjusting style and organization to be more readable and consistent
…t that were replaced with the corresponding ones from default_model_mod
…_module, adjusting style to be more readable and consistent
…ables; use len=vtablenamelength in definition for state variable_table instead of len=NF90_MAX_NAME
…default ; removed assimilation_period_days, assimilation_period_seconds from &model_nml

Set default namelist item ‘debug’ value to 1 to not print all the additional info
…tions to use the icepack model_mod. Added a example test case that uses perturb_single_instance=.true. to the repo
…etcdf variable names for the corresponding QTYs.

Note that this commit is more for bookeeping since this subroutine will be removed from the code in the following commit. Other model_mods do not include this subroutine and the default is typically hardcoded in the model_state_variables item of the &model_nml
…comment. The rest of the model_mods do not use a subroutine like this. The default is provided in the input namelist.

Added check in place of the subroutine calll to error out if the model_stat_variables nml entry is completely empty (model_stat_variables = ‘ ’)
@mjs2369 mjs2369 marked this pull request as draft October 3, 2024 15:49
@hkershaw-brown
Copy link
Member

Hi @mjs2369 not looked at this, but don't commit binary files and test data. Can you remove from the repo.
/DART/models/icepack/icepack_test (actually /DART/models/cice-scm/icepack_test

@mjs2369
Copy link
Contributor Author

mjs2369 commented Oct 3, 2024

Hi @mjs2369 not looked at this, but don't commit binary files and test data. Can you remove from the repo. /DART/models/icepack/icepack_test (actually /DART/models/cice-scm/icepack_test

yep, on it

Also meant to change the dir name but slipped my mind. I'll do that as well

Copy link
Member

@hkershaw-brown hkershaw-brown left a comment

Choose a reason for hiding this comment

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

Good start Marlee,
I've put a bunch of comments for our meeting on Monday

Cheers,
Helen

@@ -457,6 +457,7 @@ References
models/gitm/readme
models/gitm/netcdf_to_gitm_blocks
models/gitm/gitm_blocks_to_netcdf
models/icepack/readme
Copy link
Member

Choose a reason for hiding this comment

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


module model_mod

! Modules that are absolutely required for use are listed
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
! Modules that are absolutely required for use are listed

use time_manager_mod, only : time_type, set_calendar_type, get_time, set_date, get_date
use location_mod, only : location_type, get_close_type, get_close_obs, get_dist, &
convert_vertical_obs, convert_vertical_state, &
set_location, set_location_missing, VERTISLEVEL, &
Copy link
Member

Choose a reason for hiding this comment

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

set_location_missing isn't used, register_module isn't used. Have a check to see what is actually used in this module.

nc_write_model_atts, &
init_time, &
init_conditions, &
check_sfctemp_var
Copy link
Member

Choose a reason for hiding this comment

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

'check_sfctemp_var' is not a 'required by DART' routine. Is check_sfctemp_var used anywhere?

read(iunit, nml = model_nml, iostat = io)
call check_namelist_read(iunit, io, "model_nml")

call error_handler(E_MSG,'static_init_model','model_nml values are',' ',' ',' ')
Copy link
Member

Choose a reason for hiding this comment

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

what's the goal of this message? It is going to print:

'static_init_model','model_nml values are',' ',' ',' '

Comment on lines +137 to +139
model_state_variables = 'aicen', 'QTY_SEAICE_CONCENTR', 'UPDATE', 'vicen',
'QTY_SEAICE_VOLUME', 'UPDATE', 'vsnon', 'QTY_SEAICE_SNOWVOLUME',
'UPDATE'
Copy link
Member

Choose a reason for hiding this comment

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

ditto: more readable as a table

| ``analysis_mean.nc`` and ``analysis_sd`` - the mean and standard deviation of the state of all ensemble members after the assimilation
| ``obs_seq.final`` - the ensemble members' estimate of the observations.
| ``dart_log.out`` - detailed log file for the execution of filter

Copy link
Member

Choose a reason for hiding this comment

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

There is no mention of cice_to_dart in the documentation.

endif
enddo

call nc_check( nf90_open(trim(original_cice_input_file), NF90_WRITE, ncid), &
Copy link
Member

Choose a reason for hiding this comment

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

I think you could use netcdf_utilites_mod for a lot of these netcf calls.

'get_var '//trim(msgstring))


var(:) = holder(gridpt_oi,:)
Copy link
Member

Choose a reason for hiding this comment

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

is gridpt_oi a time slice?

phi ! ice liquid fraction

! from shr_const_mod.F90
real(r8),parameter :: SHR_CONST_CPSW = 3.996e3_R8 ! specific heat of sea water ~ J/kg/K
Copy link
Member

Choose a reason for hiding this comment

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

there's a mixture of r8 and R8 in this section

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.

2 participants