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

Diag_manager_mod documentation PR #199

Merged
merged 4 commits into from
Oct 7, 2021

Conversation

mp586
Copy link
Contributor

@mp586 mp586 commented Dec 16, 2020

Documentation for diag_manager_mod focusing mostly on how to save Isca output ( at different frequencies ) and including a list of standard diagnostic variables.

@mp586 mp586 added docs priority:medium Medium-piority task labels Dec 16, 2020
@rosscastle
Copy link
Contributor

Hi Marianne, sorry this took me so long to get round to! I think this is really great, there's not much I can think to improve really. A few very minor stylistic points:

  • I think it could be made a bit clearer how often you can output all the data on multiple timescales (e.g. all data hourly, daily, monthly) for 1 run. I think it would help if you explain what each of the 'module, name, time_avg, files' are - importantly make it clear that 'files' refers to the 'atmos_monthly' etc. Also you could make it clear that you only need 1. In the second sentence of the second paragraph I think it would make more sense if you started with 'If you only wanted specific variables saved on at a particular output frequency(cies), then you can...'
  • I think it would look a little neater if in the text the namelist options (e.g. true/false/none/123) were also formatted. E.g. line 81: ' If files = None ' could be ' If files = None '.
  • In the output field table, the unit for relative humidity has a \ (slash) in it.
  • Line 57, I think it would be helpful (although hopefully obvious) to point out that examples of a diag script are in those example runscripts near the top.

Copy link
Contributor

@rosscastle rosscastle left a comment

Choose a reason for hiding this comment

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

RC's changes made with MPs permission.

@rosscastle rosscastle merged commit 403f230 into ExeClim:master Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs priority:medium Medium-piority task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants