-
Notifications
You must be signed in to change notification settings - Fork 170
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 JGDAS ENKF SELECT OBS JGLOBAL_ATMOS_ANALYSIS and JGLOBAL_ATMOS_ANALYSIS_CALC jobs #3092
base: develop
Are you sure you want to change the base?
Update JGDAS ENKF SELECT OBS JGLOBAL_ATMOS_ANALYSIS and JGLOBAL_ATMOS_ANALYSIS_CALC jobs #3092
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.
looks good. I am no longer familiar w/ the IN and OUT of EnKF related data, so would appreciate a review from @CatherineThomas-NOAA @RussTreadon-NOAA
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.
Looks good! Please resolve the shellcheck warning in jobs/JGDAS_ENKF_SELECT_OBS
and then I can approve.
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.
looks good to me.
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.
Looks good, thanks @mingshichen-noaa !
Experiment C96C48_hybatmDA FAILED on Hera in Build# 1 in |
Experiment C96C48_hybatmaerosnowDA FAILED on Hera in Build# 1 with error logs:
Follow link here to view the contents of the above file(s): (link) |
Experiment C96C48_hybatmaerosnowDA FAILED on Hera in Build# 1 in |
CI Failed on Hera in Build# 1
|
@mingshichen-noaa |
ad1aa96
@aerorahul @WalterKolczynski-NOAA @KateFriedman-NOAA |
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.
Looks good for the most part. Left one comment to resolve before approving. Thanks!
@mingshichen-noaa Since this PR now also includes COM updates for the analysis job, please update the PR title and description to reflect the file changeset. Thanks! |
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.
COM updates look good, thanks @mingshichen-noaa ! Approve pending successful completion of CI testing.
@aerorahul @KateFriedman-NOAA @AntonMFernando-NOAA |
@mingshichen-noaa Thanks for checking on this! Since the change for the eobs job requires companion updates to the analysis/analysis_calc jobs at the same time then it was necessary to submit the changes together. Hopefully @AntonMFernando-NOAA hasn't already done much work on those job scripts. :) In the future, I suggest checking the spreadsheet sooner if you think you need to update other job scripts at the same time. If someone else has already assigned themself to that job then you should reach out to them to say you can take care of it within your work or collaborate with the other developer if they have already begun making changes. Hope that helps! |
@mingshichen-noaa please update the COM update tracking spreadsheet to reflect the analysis/analysis_calc job updates, thanks! |
@KateFriedman-NOAA |
@KateFriedman-NOAA @AntonMFernando-NOAA |
Description
NCO has requested that each COM variable specify whether it is an input or an output. This completes that process for the global jgdas enkf select obs job and relevant .JGLOBAL_ATMOS_ANALYSIS and JGLOBAL_ATMOS_ANALYSIS_CALC jobs.
Refs #2451
Type of change
Change characteristics
How has this been tested?
Checklist