-
Notifications
You must be signed in to change notification settings - Fork 121
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] Expand forecast fields for metric test #1048
[develop] Expand forecast fields for metric test #1048
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.
Overall, these changes look good!
However, while attempting to run the srw_metric_example.sh
script, the script failed while attempting to load the wflow_{$platform,,}
modulefile. Conda needs to be available before loading the modulefile, otherwise the script will fail. I found that moving the #build srw
section to before loading the modulefiles, the script successfully ran.
WE2E fundamental tests passed on Hera and Jet:
|
After this commands:
Shell failed with:
Script ./.cicd/scripts/srw_metric_example.sh, line 20, looks like this:
Variable workspace is getting value from SRW_PLATFORM. |
Also, variable SRW_PROJECT should be set. |
This logic was added to address shared workspaces for Gaea/Gaea-c5 and Hercules/Orion. I checked a number of T1 platforms to see if the SRW_PLATFORM directory exists and found that it only does for Gaea and Hercules/Orion. Given that this variable is a required argument, I'll change the logic to "-d" to avoid errors for non-shared workspace platforms. |
Another good find @RatkoVasic-NOAA! Somehow the experiment passed for me with no account on PW AWS. To resolve this, simply |
I tested PR on 5 machines, 3 machines passed (Hera, Jet and Hercules), and two machines failed (Orion and Gaea):
|
|
Gaea passed for me with the latest changes here: /gpfs/f5/epic/scratch/Edward.Snyder/pr_1048/ufs-srweather-app
|
Gaea worked for me as well:
|
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.
Thank you very much for working with @RatkoVasic-NOAA and me to address our concerns!
The Derecho
test successfully ran:
Skill Score: 0.98937
+ [[ 0.98937 < 0.700 ]]
+ echo 'Congrats! You pass check!'
Congrats! You pass check!
The Gaea
test successfully ran:
Skill Score: 0.98789
+ [[ 0.98789 < 0.700 ]]
+ echo 'Congrats! You pass check!'
Congrats! You pass check!
The Orion
test successfully passed:
Skill Score: 0.99043
+ [[ 0.99043 < 0.700 ]]
+ echo 'Congrats! You pass check!'
Congrats! You pass check!
Approving this PR now.
The Jet
The tests have successfully passed on Derecho, Gaea, and Hercules. The tests are still running on Hera. |
Given that Hera GNU tests are just sitting in queue for days and the inability to run Hera GNU on Rocky8, the successful run of the Hera Intel will be enough to get this work merged. Once HPSS has returned to service following maintenance, I will manually run the Jenkins coverage tests on Hera Intel and post the summary in this PR. There was a failure in the |
The Hera Intel coverage WE2E tests were successfully run using Rocky8:
The Orion tests are still sitting in queue, so will continue to hold off until the tests on Orion are complete and final check with @EdwardSnyder-NOAA before merging this PR. |
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.
While running the srw_metric_example.sh
script, the WE2E test failed to run, leading to the test failing. It looks like RUN_WE2E_OPTi should be replaced with RUN_WE2E_OPT.
The Jenkins tests failed to kick off the WE2E coverage tests on Orion, so I manually ran them and they all passed:
I'm running one last test on the latest update to the |
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 correcting the typos! My retest successfully passed:
Skill Score: 0.99043
+ [[ 0.99043 < 0.700 ]]
+ echo 'Congrats! You pass check!'
Congrats! You pass check!
So I will now reapprove this PR and get it merged.
DESCRIPTION OF CHANGES:
This PR expands the number of forecast fields for the Skill Score metric test. The forecast length in the metric WE2E test was extended to 12 hours so that the RMSE metric can be calculated for these additional forecast fields:
Adding these additional forecast fields will make the skill score metric test more thorough and thus making it a more inclusive test to compare against.
Also, a change was made to the
.cicd/scripts/srw_metric_example.sh
script to reflect the new conda environment.Type of change
TESTS CONDUCTED:
Those interested in running the
.cicd/scripts/srw_metric_example.sh
will need to do the following below. Note, this script builds the app, so this process can run after runningmanage_externals
../.cicd/scripts/srw_metric_example.sh
DEPENDENCIES:
DOCUMENTATION:
ISSUE:
CHECKLIST
LABELS (optional):
A Code Manager needs to add the following labels to this PR:
CONTRIBUTORS (optional):