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

Changed default namelist and streams to E3SM default #706

Open
wants to merge 4 commits into
base: seaice/develop
Choose a base branch
from

Conversation

akturner
Copy link
Collaborator

Changed Registry.xml configs and streams to E3SM default
Added forcing directory to forcing filenames in Registry
Updated SnowAgingPropertiesInput file from snicar_drdt_bst_fit_60_c04182019.nc to snicar_drdt_bst_fit_60_c04262019.nc
Modified testing to allow exceptions in individual tests without then stopping and allowing remaining tests to run
Added e3sm default configuration to configurations
Added e3sm default configuration to tests

Changed Registry.xml configs and streams to E3SM default
Added forcing directory to forcing filenames in Registry
Updated SnowAgingPropertiesInput file from snicar_drdt_bst_fit_60_c04182019.nc to snicar_drdt_bst_fit_60_c04262019.nc
Modified testing to allow exceptions in individual tests without then stopping and allowing remaining tests to run
Added e3sm default configuration to configurations
Added e3sm default configuration to tests
@akturner
Copy link
Collaborator Author

Note file change for SnowAgingPropertiesInput

@njeffery
Copy link
Contributor

@akturner, the namelist for e3sm_default still has the old default values. The streams file looks okay.

@akturner
Copy link
Collaborator Author

@njeffery: Thanks for noticing that - I will fix ASAP.

Fixed minor bug in testing
Added correct e3sm_default namelist and streams - fails restartability test
@akturner
Copy link
Collaborator Author

@njeffery, @proteanplanet: With the e3sm default options the restartability test failed, so maybe a variable is missing from restart? Any ideas what that would be?

@proteanplanet
Copy link

@njeffery, @proteanplanet: With the e3sm default options the restartability test failed, so maybe a variable is missing from restart? Any ideas what that would be?

@akturner Thanks for trying to get this through. I'll take a look at this next week when V2 is out of the way. For the record, I've done an ERS test within E3SM on G-cases with these default settings, and it passed, so a quick comparison between the two configurations should reveal the problem.

@njeffery
Copy link
Contributor

@akturner : there are two config options that need to be turned on during restarts: config_do_restart_snow_density = true and config_do_restart_snow_grain_radius = true. The testsuite.snow_tracer_physics.xml has them included with the option "snow_tracer_physics" value="True".

Passes all tests except expected fail for e3sm_default regression
@akturner
Copy link
Collaborator Author

@njeffery , @proteanplanet : I've fixed the restartability testing issue

Copy link

@proteanplanet proteanplanet left a comment

Choose a reason for hiding this comment

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

Since this passes all tests, and all namelist options for the default E3SM V2 configuration are switched on, I approve this request. Thanks @akturner!

@njeffery
Copy link
Contributor

@akturner : While testing on anvil, I found a couple of things that you may want to roll into this PR. The get_data.py script has old python 2 syntax that no longer works with python 3. string.strp should be changed to str.strp. Another really minor thing, the testing readme file describing set-up of the mpas_seaice conda environment should have conda install -n mpas_seaice -c anaconda six

Copy link
Contributor

@njeffery njeffery left a comment

Choose a reason for hiding this comment

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

Looks good. Passes parallelism and restartability. I'll add a consistency check in MPAS for snow restart flags in a later PR.

Removed support for python 2
Removed need for six module
Removed string module usage
@akturner
Copy link
Collaborator Author

@njeffery I removed python 2 support from the script, removing the string and six module usage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants