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

feature/dev_evs_v1.0.17 #658

Merged

Conversation

malloryprow
Copy link
Contributor

Note to developers: You must use this PR template!

Description of Changes

This brings in the changes from the EVS v1.0.17 implementation into develop.

Developer Questions and Checklist

  • Is this a high priority PR? If so, why and is there a date it needs to be merged by?

Yes, so the wafs stats job won't fail

  • Do you have any planned upcoming annual leave/PTO?

No

  • Are there any changes needed in the times when the jobs are supposed to run/kick-off?

No

  • The code changes follow NCO's EE2 Standards.
  • Developer's name is removed throughout the code and have used ${USER} where necessary throughout the code.
  • References the feature branch for HOMEevs are removed from the code.
  • J-Job environment variables, COMIN and COMOUT directories, and output follow what has been defined for EVS.
  • Jobs over 15 minutes in runtime have restart capability.
  • If applicable, changes in the dev/drivers/scripts or dev/modulefiles have been made in the corresponding ecf/scripts and ecf/defs/evs-nco.def?
  • Jobs contain the appropriate file checking and don't run METplus for any missing data.
  • Code is using METplus wrappers structure and not calling MET executables directly.
  • Log is free of any ERRORs or WARNINGs.

Testing Instructions

Set-up

  1. Clone my fork and checkout branch feature/dev_evs_v1.0.17
  2. ln -sf /lfs/h2/emc/vpppg/noscrub/emc.vpppg/verification/EVS_fix fix

🔲 wafs stats

3.cd dev/drivers/scripts/stats/wafs
4. Run jevs_wafs_atmos_stats.sh

@AliciaBentley-NOAA
Copy link
Contributor

@malloryprow The changes made in https://github.com/NOAA-EMC/EVS/pull/577/files do not appear as changed code in this update. Should they? Or where they already there in develop?

@AliciaBentley-NOAA
Copy link
Contributor

@malloryprow The changes made in https://github.com/NOAA-EMC/EVS/pull/579/files also do not appear as changed code in this update. Should they? Or were they already there in develop?

@AliciaBentley-NOAA
Copy link
Contributor

@malloryprow The changes made in https://github.com/NOAA-EMC/EVS/pull/581/files also do not appear as changed code in this update. Should they? Or were they already there in develop?

@malloryprow
Copy link
Contributor Author

All the plot unit conversion fixes are in develop since the images coming from the emc.vpppg parallel are correct.

#572 #526 #521

@AliciaBentley-NOAA
Copy link
Contributor

@malloryprow Even weirder is that only some of the WAFS changes made it into this feature branch. The only one that appears to be missing are the changes made to jobs/JEVS_WAFS_ATMOS_STATS in https://github.com/NOAA-EMC/EVS/pull/576/files.

I feel like having none of the WAFS updates or all of the WAFS updates in a PR makes sense, but not missing one of the updates! Should we make the changes to jobs/JEVS_WAFS_ATMOS_STATS?

@malloryprow
Copy link
Contributor Author

What changes are missing from JEVS_WAFS_ATMOS_STATS? I see the two line changes in this PR.

@AliciaBentley-NOAA
Copy link
Contributor

@malloryprow JEVS_WAFS_ATMOS_STATS in the original WAFS PR seems to have become JEVS_WAFS_STATS in develop. I see the changes now, too!

@AliciaBentley-NOAA AliciaBentley-NOAA self-requested a review January 29, 2025 13:44
Copy link
Contributor

@AliciaBentley-NOAA AliciaBentley-NOAA left a comment

Choose a reason for hiding this comment

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

I have reviewed all of the changes made in this PR very thoroughly and can confirm that the changes made to the EVS v1.0.17 release are included in this PR. I approve this PR to be merged after necessary testing is complete.

CC @PerryShafran-NOAA @malloryprow

@PerryShafran-NOAA
Copy link
Contributor

This PR is still in a draft form. Should I begin testing anyway?

@malloryprow
Copy link
Contributor Author

Please wait until I can confirm that WAFS v7.0 went in. I emailed NCO about it but haven't heard. Once I know it is I'll move it out of draft.

@PerryShafran-NOAA
Copy link
Contributor

OK, I'll wait to test until you give me the go ahead.

@malloryprow malloryprow marked this pull request as ready for review January 29, 2025 17:21
@malloryprow
Copy link
Contributor Author

Good to test!

@PerryShafran-NOAA
Copy link
Contributor

Test underway!

@AliciaBentley-NOAA
Copy link
Contributor

@PerryShafran-NOAA @malloryprow I got the following missing file message from Perry's test.

WARNING: A subset of blend forecasts ( F18) is missing for GCIP valid date 2025012800. METplus continues.
Job ID: jevs_wafs_stats.178331765.cbqs01

@PerryShafran-NOAA
Copy link
Contributor

@AliciaBentley-NOAA @malloryprow The missing file is:

WARNING: missing forecast /lfs/h1/ops/prod/com/wafs/v7.0/wafs.20250127/06/grib2/0p25/blending/WAFS_0p25_blended_2025012706f018.grib2
0 + echo 'WARNING: missing forecast /lfs/h1/ops/prod/com/wafs/v7.0/wafs.20250127/06/grib2/0p25/blending/WAFS_0p25_blended_2025012706f018.grib2'

Interestingly, the wafs.20250127 directory is the only one that has missing data in there. Due to the lack of any forecasts for 20250127, there are several WARNINGs throughout the output file. There are no ERRORs or anything else out of whack, and there are final stat files in the COMOUTfinal directory. Thus I think this is OK.

@PerryShafran-NOAA
Copy link
Contributor

@malloryprow The job is finished:

.o file: /lfs/h2/emc/vpppg/noscrub/perry.shafran/pr658test/EVS/dev/drivers/scripts/stats/wafs/jevs_wafs_stats.o178331765
small stats files: /lfs/h2/emc/vpppg/noscrub/perry.shafran/evs/v2.0/stats/wafs/atmos.20250128/wafs/grid2grid
final stats files: /lfs/h2/emc/vpppg/noscrub/perry.shafran/evs/v2.0/stats/wafs/wafs.20250128
working directory: /lfs/h2/emc/stmp/perry.shafran/evs_test/prod/tmp/jevs_wafs_stats.178331765.cbqs01

@malloryprow
Copy link
Contributor Author

May need to keep an eye on if the missing file emails persist going forward.

The evs.stats.wafs.atmos.grid2grid_uvt1p25.v20250128.stat matches the parallel but evs.stats.wafs.atmos.grid2grid_icesev.v20250128.stat is different however this is expected. @YaliMao-NOAA told me this when she reviewed the files from the NCO para testing.

@PerryShafran-NOAA
Copy link
Contributor

So wanted to be sure that you validated the results of this test, or would you care for another test for a different date?

@YaliMao-NOAA
Copy link
Contributor

May need to keep an eye on if the missing file emails persist going forward.

The evs.stats.wafs.atmos.grid2grid_uvt1p25.v20250128.stat matches the parallel but evs.stats.wafs.atmos.grid2grid_icesev.v20250128.stat is different however this is expected. @YaliMao-NOAA told me this when she reviewed the files from the NCO para testing.

Yes, icesev stat file is different because the validation data GCIP is different which ingests METAR observation data, while METAR observation data keeps on accumulation dynamically over time.

I will evaluate WAFS EVS tomorrow which runs once per day, while WAFS just got implemented 12z today.

@malloryprow
Copy link
Contributor Author

I believe we can merge this.

Copy link
Contributor

@PerryShafran-NOAA PerryShafran-NOAA left a comment

Choose a reason for hiding this comment

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

Changes work as expected. Approved for merge.

@PerryShafran-NOAA PerryShafran-NOAA merged commit ce971f5 into NOAA-EMC:develop Jan 30, 2025
@YaliMao-NOAA
Copy link
Contributor

YaliMao-NOAA commented Jan 30, 2025

@malloryprow @PerryShafran-NOAA
From 01/30/2025, parallel WAFS EVS was switched to WAFS operational data, the differences between parallel and operational WAFS EVS icesev stat files were gone.

@malloryprow malloryprow deleted the feature/dev_evs_v1.0.17 branch January 31, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update model or library updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants