-
Notifications
You must be signed in to change notification settings - Fork 119
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
[develop]: Updated ConfigWorkflow.rst to reflect changes to config_defaults.yaml (PI13) #1133
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for going through the ConfigWorkflow.rst
documentation and updating the entries that have been updated in config_defaults.yaml
!
Cross referencing the changes that you have made in this PR with what is currently in the ConfigWorkflow.rst
file at the HEAD of the develop branch, it looks like several of these entries are currently in the documentation. Looking at the commit history for your text/us-336
branch, it looks like the branch was created from a February 21, 2024 hash, with no other syncing with the develop branch after it was created.
Please merge the authoritative develop branch back into your text/us-336
branch. If you have any questions or require assistance, please let me know.
@gspetro-NOAA - While looking through the ConfigWorkflow.rst
file and config_defaults.yaml
, specifically in the FIX*
sections, there is a disconnect occurring. In ConfigWorkflow.rst
, the definition of variables is path to system directory
, while in config_defaults.yaml
, it is system directory where
.
Should the definitions be updated in either the ConfigWorkflow.rst
or config_defaults.yaml
files (and which file should be updated, if you think it should), or do you feel that the differences for the FIX*
variables are fine as is?
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
doc/UsersGuide/source/CustomizingTheWorkflow/ConfigWorkflow.rst
Outdated
Show resolved
Hide resolved
@jdkublnick I think the only changes required are from PR #1098. Pretty much everything else should remain as-is. |
@MichaelLueken I think "Path to system directory..." is more accurate than "System directory where...". As far as I can tell, users would need to add the full path to the directory, not just the name of the directory, and the "Path to..." makes that clearer imo. Looking at, e.g., the Hera machine file, |
Thanks for the explanation, @gspetro-NOAA! I'm fine with keeping |
Co-authored-by: Gillian Petro <[email protected]>
Co-authored-by: Gillian Petro <[email protected]>
Co-authored-by: Gillian Petro <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for merging the latest develop into text/us-336! The OMP_NUM_THREADS_RUN_FCST
default value should be 2, rather than 1.
Co-authored-by: Michael Lueken <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again, @jdkublnick!
The differences between the config_defaults.yaml
and ConfigWorkflow.rst
files appear to be addressed now.
Co-authored-by: Gillian Petro <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested changes were made. Looks good to me.
Moving forward with merging this work now. |
DESCRIPTION OF CHANGES:
Updated ConfigWorkflow.rst to reflect recent changes to config_defaults.yaml in order to keep documentation up to date.
Type of change
[X ] This change requires a documentation update
TESTS CONDUCTED:
None required for documentation. The docs build successfully on my fork and can be viewed at: https://ufs-srweather-app-joshua-kublnick.readthedocs.io/en/text-us-336/BackgroundInfo/index.html
DEPENDENCIES:
N/A
DOCUMENTATION:
All documentation
ISSUE:
Issue #1132
CHECKLIST
[X ] My code follows the style guidelines in the Contributor's Guide
[X ] I have performed a self-review of my own code using the Code Reviewer's Guide
I have commented my code, particularly in hard-to-understand areas
[X ] My changes need updates to the documentation. I have made corresponding changes to the documentation
My changes do not require updates to the documentation (explain).
[X ] My changes generate no new warnings
New and existing tests pass with my changes
[ X] Any dependent changes have been merged and published
LABELS (optional):
A Code Manager needs to add the following labels to this PR:
[X ] documentation