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

Update JRA55 forcing implementation #562

Merged
merged 12 commits into from
Feb 12, 2021
Merged

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Feb 5, 2021

PR checklist

  • Short (1 sentence) summary of your PR:
    Update JRA55 forcing implementation
  • Developer(s):
    apcraig
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    Full test suite run on cheyenne with 3 compilers, all results change (as expected) https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#96a5103a495df1caa5703c478f143a4bc81cde79
    Also ran 10+ years for 1995-2005 on cheyenne with debugging output on (for part of the time). As far as I can tell, things are working correctly.
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

Several bug fixes addressed

  • Reading two timesteps instead of 1 now (was reading the same timestep into both slots)
  • Advancing files by year (was just cycling the initial year in a multi-year run)
  • Time interpolation updated (was not working in multi-year runs)
  • Interpolate 4 input states in time, but hold 3 input fluxes constant over 3 hour forcing period
  • Additional output diagnosing forcing filename as it evolves

See #249, #343, #482, #558

Several bug fixes addressed
- Reading two timesteps instead of 1 now (was reading the same timestep into both slots)
- Advancing files by year (was just cycling the initial year in a multi-year run)
- Time interpolation updated (was not working in multi-year runs)
- Interpolate 4 input states in time, but hold 3 input fluxes constant over 3 hour forcing period
- Additional output diagnosing forcing filename as it evolves
@apcraig
Copy link
Contributor Author

apcraig commented Feb 5, 2021

A couple questions/comments.

  • Do we want to run a QC? It seems to me we know this fixes bugs, I'm not sure we care whether QC passes or not.
  • There is no documentation on "forcing" in the user guide. Should we add a section? If so, I can add something to this PR, but maybe we need an issue to address the forcing documentation more generally.
  • The 10 year run on cheyenne is here,
    cheyenne: /glade/scratch/tcraig/CICE_RUNS/jragx1test_tc03
    if anyone wants to look at it.

@eclare108213
Copy link
Contributor

I don't think we need QC for this one. The important thing is to get this fixed quickly. I think there is some forcing documentation, somewhere, since we reference the data sets, but it might not explain how the forcing module/routines work. That information should be added, preferably in this PR unless it'll take a long time -- I'd be fine with addressing that as a separate issue, e.g. as part of a 'clean up' phase.
Does it make sense to run the cesm sea ice diagnostics on the cheyenne run, @dabail10 ?

@daveh150
Copy link
Contributor

The code changes Tony committed look good. I like putting the reads in a for loop.

Were we going to standardize the JRA55 forcing names to include the grid. So it would be

JRA55_03hr_forcing_gx3_2005.nc
JRA55_03hr_forcing_gx1_2005.nc
JRA55_03hr_forcing_tx1_2005.nc

If we do that, I think we can remove the hardcoded '/8XDAILY/ ' part. That should be defined in atm_data_dir namelist varaible.

@rallard77
Copy link
Contributor

rallard77 commented Feb 10, 2021 via email

Copy link
Contributor

@daveh150 daveh150 left a comment

Choose a reason for hiding this comment

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

Approved as is submitted. As discussed on today's Telecon the JRA55 file naming standard will be updated in future PR.

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

I like the new forcing documentation - very helpful. Made a few suggestions there.

doc/source/developer_guide/dg_forcing.rst Show resolved Hide resolved
doc/source/developer_guide/dg_forcing.rst Outdated Show resolved Hide resolved
doc/source/developer_guide/dg_forcing.rst Outdated Show resolved Hide resolved
The ``default`` ocean setting is the standard setting used in standalone CICE runs.
In this mode, the sea surface salinity is set to 34 ppt and the sea surface
temperature is set to the freezing temperature at all grid points and
held constant. Other ocean coupling fields are set to zero. No files are read.
Copy link
Contributor

Choose a reason for hiding this comment

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

held constant unless the mixed layer parameterization is turned on, in which case the SST evolves

Copy link
Contributor

Choose a reason for hiding this comment

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

and when the mixed layer parameterization is on, it can use data from the ocean forcing file, which we only have for gx1. It might not be used in our testing and might not be offered as a forcing data set...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the text a bit, but did not add the stuff about an ocean forcing file. I don't think we test anything other than the default, and I don't think we've released any ocean forcing dataset.

@apcraig apcraig merged commit d9a9f6d into CICE-Consortium:master Feb 12, 2021
This was referenced Feb 27, 2021
@apcraig apcraig deleted the jra55fixA branch August 17, 2022 21:00
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