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.

Sign up for GitHub

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

Jump to bottom

Add JRA55 dataset tool and documentation #582

Merged
merged 3 commits into from
Mar 25, 2021
Merged

Add JRA55 dataset tool and documentation #582

merged 3 commits into from
Mar 25, 2021

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Mar 23, 2021

PR checklist

  • Short (1 sentence) summary of your PR:
    Add JRA55 dataset tool

  • Developer(s):
    apcraig, dabail10, rallard77, daveh150

  • 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.
    Only documentation/tools changes, no model testing. Did test the JRA55 tool and verified output is bit-for-bit with @dabail10 output.

  • 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:

  • Add configuration/tools/jra55_datasets directory and scripts

  • Move convert_restarts to configuration/tools/cice4_restart_conversio

  • Add tools section to the developer guide

Unrelated to tools

  • Update tested compiler versions
  • Change "compliance" to "validation" in documentation when discussing QC testing.

@apcraig
Add JRA55 dataset tool
d4dfb5a 
Add configuration/tools/jra55_datasets directory and scripts
Move convert_restarts to configuration/tools/cice4_restart_conversio
Add tools section to the developer guide

Update tested compiler versions
Change "compliance" to "validation" in documentation when discussing QC testing.
@apcraig
Copy link
Contributor Author

apcraig commented Mar 23, 2021

I will need to review the documentation once generated and probably need to do some cleanup.

I leveraged much of the existing documentation (in Word), but didn't bring in all the figures and some other details. Please let me know if you feel anything is missing! I'm happy to revise a few times.

eclare108213
eclare108213 requested changes Mar 23, 2021
View reviewed changes
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.

@apcraig this looks great. Thanks also @daveh150 and @dabail10. A few comments and questions below -

doc/source/developer_guide/dg_scripts.rst Outdated
when the compliance test or compliance test scripts (i.e., ``cice.t-test.py``) are modified.
Again, this section **documents a validation process for the compliance scripts**; it does not
describe to how run the compliance test itself.
The code validation (QC) test validates non bit-for-bit model changes. The directory
Copy link
Contributor

Choose a reason for hiding this comment

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

Define what QC is, if it's not defined above, e.g. "The code validation (Quality Control, QC) test..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I have updated the documentation to mix validation and QC better. I was looking at the documentation and got confused about the word compliance. "code compliance" sounded like coding syntax, format, interfaces, and similar. That's why I switched to "code validation".

doc/source/developer_guide/dg_scripts.rst Outdated

Two scripts have been created to automatically validate the code compliance script.
Two scripts have been created to automatically validate the code validation script.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of "validate the code validation script", maybe "validate the QC script"? This is probably why we used 'compliance' before. (also in other files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above.

doc/source/developer_guide/dg_tools.rst Show resolved Hide resolved
doc/source/developer_guide/dg_tools.rst Outdated

The "Data access" tabs will provide a list of product categories.
The JRA55 data of interest are located in 2 separate products. Winds, air
temperature, and specific humidity on the model grid are included in "JRA-55
Copy link
Contributor

Choose a reason for hiding this comment

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

not on the model grid... on the JRA55 grid?

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 cleaned that up.

doc/source/developer_guide/dg_tools.rst Show resolved Hide resolved
@apcraig
Copy link
Contributor Author

apcraig commented Mar 23, 2021

The updated documentation appears here in readthedocs, https://cice-consortium-cice--582.org.readthedocs.build/en/582/developer_guide/dg_tools.html.

I do have one question for @dabail10, @rallard77, @daveh150 with regard to requirements. The word document mentioned argparse and datetime as required python packages. Is that still the case. When I look at my packages on cheyenne, I don't actually see those listed. Are any other packages needed that are not listed in the document? This is what we have now, I'd like to make sure we have the list correct.

  • python3 (python3.7.9)
  • numpy (1.18.5)
  • netCDF4 (1.5.5)
  • ESMPy (8.0.0)
  • xesmf (0.3.0)
  • argparse
  • datetime

@daveh150
Copy link
Contributor

daveh150 commented Mar 24, 2021 via email

@daveh150
Copy link
Contributor

daveh150 commented Mar 24, 2021 via email

@dabail10
Copy link
Contributor

Also, this version of the code does not use ESMPy. It only uses xESMF. I'll have more of a look at the documentation.

@daveh150
Copy link
Contributor

daveh150 commented Mar 24, 2021 via email

@apcraig
Copy link
Contributor Author

apcraig commented Mar 25, 2021

Thanks @daveh150, you are correct that prepare_forcing disaggregates the precip to rain or snow depending on Tair, so that is good. I will update the documentation.

@apcraig
Copy link
Contributor Author

apcraig commented Mar 25, 2021

I have updated the documentation to clarify rain/snow and remove argparse and datetime from explicitly listed packages. Any other feedback?

eclare108213
eclare108213 approved these changes Mar 25, 2021
View reviewed changes
doc/source/developer_guide/dg_tools.rst
- All files should be placed in a common directory. This includes the raw JRA55 input files, the CICE grid file, and **interp_jra55_ncdf_bilinear.py**. The output files will be written to the same directory.
- The script **configuration/tools/jra55_datasets/make_forcing.csh** was used on the NCAR cheyenne machine in March, 2021 to generate CICE forcing data. It assumes the raw JRA55 is downloaded, but then sets up the python environment, links all the data in a common directory, runs **interp_jra55_ncdf_bilinear.py** and then aggregates the quarterly data using NCO.
- The new forcing files can then be defined in the **ice_in** namelist file using the input variables, ``atm_data_type``, ``atm_data_format``, ``atm_data_dir``, ``fyear_init``, and ``ycycle``. See :ref:`forcing` for more information.
- The total precipitation field is mm/day in JRA55. This field is initially read in as snow, but prepare_forcing in **ice_forcing.F90** splits that into rain or snow forcing depending on the air temperature.
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, it's probably mm/day of liquid water (rain), which is read into a variable called 'fsnow' before being adjusted with snow density in the forcing module. But that's a minor quibble.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like both snow and rain are in units of kg/m2-s in the forcing. I assume that water flux is converted to a snow depth output the forcing module?

@daveh150
Copy link
Contributor

daveh150 commented Mar 25, 2021 via email

@apcraig apcraig merged commit e204fb8 into CICE-Consortium:master Mar 25, 2021
@eclare108213 eclare108213 mentioned this pull request May 20, 2021
Closed
@apcraig apcraig deleted the jra55_tool branch August 17, 2022 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eclare108213 eclare108213 eclare108213 approved these changes

@dabail10 dabail10 Awaiting requested review from dabail10

@daveh150 daveh150 Awaiting requested review from daveh150

@rallard77 rallard77 Awaiting requested review from rallard77

Assignees
No one assigned
Labels
Documentation Tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@apcraig @daveh150 @dabail10 @eclare108213