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

adding CMIP variable documentation #195

Merged
merged 4 commits into from
Oct 5, 2018
Merged

adding CMIP variable documentation #195

merged 4 commits into from
Oct 5, 2018

Conversation

duvivier
Copy link
Contributor

@duvivier duvivier commented Oct 1, 2018

Documentation about addition of CMIP6/SIMIP variable inclusion in history variables. Follow up to PR #191

  • Developer(s): Alice DuVivier

  • Please suggest code Pull Request reviewers in the column at right.

  • Are the code changes bit for bit, different at roundoff level, or more substantial? bfb

  • Please include the link to test results or paste the summary block from the bottom of the testing output below. n/a

  • Does this PR create or have dependencies on Icepack or any other models? no

  • Is the documentation being updated with this PR? Y - follow up to Update CICE with CMIP changes. #191
    If not, does the documentation need to be updated separately at a later time? (Y/N)

Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.

  • Other Relevant Details:

@duvivier duvivier self-assigned this Oct 1, 2018
dabail10
dabail10 previously approved these changes Oct 1, 2018
Copy link
Contributor

@dabail10 dabail10 left a comment

Choose a reason for hiding this comment

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

I think this looks good. Thanks for taking care of this! It would be nice to point to the SIMIP paper on this here:

https://www.geosci-model-dev.net/9/3427/2016/

eclare108213
eclare108213 previously approved these changes Oct 2, 2018
@duvivier duvivier dismissed stale reviews from eclare108213 and dabail10 via 146f9f2 October 2, 2018 02:21
@duvivier
Copy link
Contributor Author

duvivier commented Oct 2, 2018

@dabail10 I added the correct reference for SIMIP you suggest.

@eclare108213 @dabail10 (@proteanplanet ). I noticed some inconsistencies/duplicates in the bibliography while adding the above reference. I have also fixed these and the relevant citations in text. We have two references in master_list.bib that are unused anywhere in the documentation- Roberts2018 and Irving2015. It appears these were added as part of #43 from last November and are related to testing developed by Andrew but was a commit by @dabail10.
@proteanplanet , if you are able to quickly peruse the section that was added with #43 (https://cice-consortium-cice.readthedocs.io/en/master/user_guide/ug_testing.html#two-stage-paired-thickness-test) and check if these references are indeed missing and should be added, that would be helpful.

Finally, I just cleaned up some other formatting bugs that were giving errors in sphinx and causing minor oddities in the documentation.

Copy link
Contributor

@dabail10 dabail10 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. We'll see what Andrew has to say about those references.

@eclare108213 eclare108213 merged commit df1b09d into CICE-Consortium:master Oct 5, 2018
@duvivier duvivier deleted the doc-191 branch October 8, 2018 21:19
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