Fix unit conversion logic

MarcelCaron-NOAA commented 1 month ago

Note to developers: You must use this PR template!

Description of Changes

Please include a summary of the changes and the related GitHub issue(s). Please also include relevant motivation and context.

This fix matches the fix made in PR #521. It addresses a logic error affecting plotting output for five EVS components. The details are reprinted here.

The logic error was found in several instances of code used for converting the units of certain METplus summary statistics. Values of these stats were already converted before being used in follow-up conversion equations (incorrect). As a result, some output plots were displaying incorrect "doubly converted" values of verification metrics.

In this fix, METplus summary statistics are first stored as new variables prior to conversion, and these new variables are used when performing unit conversion and are never overwritten. As a result, all EVS plots affected by the issue should display correct converted values of verification metrics.

Developer Questions and Checklist

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

Yes. Currently, some EVS plots are inaccurate if they display SL1L2- or VL1L2-based metrics and apply unit conversion. The issue affects five EVS components.

Do you have any planned upcoming annual leave/PTO?

No. My work schedule may be impacted by the Hurricane Helene recovery timeline (will edit when this is no longer the case).

Are there any changes needed for when the jobs are supposed to run?

No.

[x] The code changes follow NCO's EE2 Standards.
[x] Developer's name is removed throughout the code and have used ${USER} where necessary throughout the code.
[x] References the feature branch for HOMEevs are removed from the code.
[x] J-Job environment variables, COMIN and COMOUT directories, and output follow what has been defined for EVS.
[x] Jobs over 15 minutes in runtime have restart capability.
[x] 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?
[x] Jobs contain the approriate file checking and don't run METplus for any missing data.
[x] Code is using METplus wrappers structure and not calling MET executables directly.
[x] Log is free of any ERRORs or WARNINGs.

Testing Instructions

Please include testing instructions for the PR assignee. Include all relevant input datasets needed to run the tests.

(1) Set up jobs

symlink the EVS_fix directory locally as "fix"
In each driver script, edit the following environment variables: HOMEevs - set to your test EVS directory COMIN - set to /lfs/h1/ops/prod/com/$NET/$evs_ver_2d COMOUT - set to your test output directory SENDMAIL - (optional) set to "NO"

(2) Running jobs

I recommend testing the following jobs:

jevs_cam_grid2obs_plots
jevs_mesoscale_grid2obs_plots
jevs_global_ens_atmos_gefs_grid2obs_past31days_plots
jevs_analyses_grid2obs_plots
jevs_aqm_grid2obs_plots

[Total: 5 plots jobs]

All driver scripts can be submitted using qsub -v vhr=00 $driver
All jobs can be submitted at any time

(3) Checking jobs

Log files should be checked by the developer for the following keywords:

check="FATAL\|WARNING\|error\|Killed\|Cgroup\|argument expected\|No such file\|cannot\|failed\|unexpected\|exceeded"
grep "$check" $logfile

Additionally, a sample of test output plots should be compared to EVS prod output.

Note: The changes in this PR affect unit conversion, so plots with no unit conversion will look the same. Whether or not a plot is affected also depends on the metric shown; metrics computed with ffbar, oobar, or fobar are affected (e.g., rmse), but metrics computed with only fbar and obar are not (e.g., bias).

malloryprow commented 1 month ago

jevs_cam_grid2obs_plots

Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/cam/jevs_cam_grid2obs_plots.o157072007 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0/plots/cam DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_cam_grid2obs_plots.157072007.cbqs01

malloryprow commented 1 month ago

jevs_mesoscale_grid2obs_plots

Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/mesoscale/jevs_mesoscale_grid2obs_plots.o157073250 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0/plots/mesoscale DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_mesoscale_grid2obs_plots.157073250.cbqs01

Tagging @PerryShafran-NOAA for awareness

malloryprow commented 1 month ago

jevs_global_ens_atmos_gefs_grid2obs_past31days_plots

Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/global_ens/jevs_global_ens_atmos_gefs_grid2obs_past31days_plots.o157073480 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0 DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_global_ens_atmos_gefs_grid2obs_past31days_plots.157073480.cbqs01

Tagging @GwenChen-NOAA for awareness

malloryprow commented 1 month ago

~jevs_analyses_grid2obs_plots~

~Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/analyses/jevs_analyses_grid2obs_plots.o157073976 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0 DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_analyses_grid2obs_plots.157073976.cbqs01~

~Tagging @PerryShafran-NOAA for awareness~

malloryprow commented 1 month ago

jevs_aqm_grid2obs_plots

Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/aqm/jevs_aqm_grid2obs_plots.o157074109 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0 DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_aqm_grid2obs_plots.157074109.cbqs01

Tagging @Ho-ChunHuang-NOAA for awareness

MarcelCaron-NOAA commented 1 month ago

The following completed cleanly and output looks good: ✔️ jevs_global_ens_atmos_gefs_grid2obs_past31days_plots

The following failed but seems to just need COMIN corrected: ⚠️ jevs_analyses_grid2obs_plots

export COMIN=/lfs/h1/ops/prod/com/${NET}/${evs_ver_2d} (remove \"$USER\")

Still running: ⏳ jevs_cam_grid2obs_plots ⏳ jevs_mesoscale_grid2obs_plots ⏳ jevs_aqm_grid2obs_plots

malloryprow commented 1 month ago

jevs_analyses_grid2obs_plots

Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/analyses/jevs_analyses_grid2obs_plots.o157073976 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0 DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_analyses_grid2obs_plots.157073976.cbqs01

Tagging @PerryShafran-NOAA for awareness

Rerun with correct COMIN

jevs_analyses_grid2obs_plots

Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/analyses/jevs_analyses_grid2obs_plots.o157075692 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0 DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_analyses_grid2obs_plots.157075692.cbqs01

Tagging @PerryShafran-NOAA for awareness

GwenChen-NOAA commented 1 month ago

jevs_global_ens_atmos_gefs_grid2obs_past31days_plots

Log File: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/EVS/dev/drivers/scripts/plots/global_ens/jevs_global_ens_atmos_gefs_grid2obs_past31days_plots.o157073480 COMOUT: /lfs/h2/emc/vpppg/noscrub/mallory.row/verification/EVS_PRs/pr579/evs/v1.0 DATA: /lfs/h2/emc/stmp/mallory.row/evs_test/prod/tmp/jevs_global_ens_atmos_gefs_grid2obs_past31days_plots.157073480.cbqs01

Tagging @GwenChen-NOAA for awareness

Plots look good to me.

Ho-ChunHuang-NOAA commented 1 month ago

The AQM plots looks good to me

https://www.emc.ncep.noaa.gov/users/verification/air_quality/aqm/pr/grid2obs/pm25/

MarcelCaron-NOAA commented 1 month ago

Thanks @Ho-ChunHuang-NOAA!
✔️ jevs_aqm_grid2obs_plots

⏳ jevs_cam_grid2obs_plots ⏳ jevs_mesoscale_grid2obs_plots ⏳ jevs_analyses_grid2obs_plots

AliciaBentley-NOAA commented 1 month ago

@MarcelCaron-NOAA @Ho-ChunHuang-NOAA Thanks! Can you please remind us how this particular PR should impact AQM? I was thinking that this PR primarily fixed temperature and dewpoint units on plots. Is there an AQM plot that should have changed?

@malloryprow @AndrewBenjamin-NOAA

MarcelCaron-NOAA commented 1 month ago

@MarcelCaron-NOAA @Ho-ChunHuang-NOAA Thanks! Can you please remind us how this particular PR should impact AQM? I was thinking that this PR primarily fixed temperature and dewpoint units on plots. Is there an AQM plot that should have changed?

@malloryprow @AndrewBenjamin-NOAA

@AliciaBentley-NOAA That's right, no aqm plot should change. The changes in this PR affect unit conversion, so plots with no unit conversion will look the same. It also depends on the metric shown; metrics computed with ffbar, oobar, or fobar are affected (e.g., rmse), but metrics computed with only fbar and obar are not (e.g., bias).

Even though unit conversion is not currently used in aqm plots, the same faulty unit conversion logic exists and is corrected here.

malloryprow commented 1 month ago

It should be the exact same thing as ops, but a script was edited so good to test to make sure it didn't introduced anything unwanted.

AliciaBentley-NOAA commented 1 month ago

That makes sense. I found that the test plots and the prod plots for AQM looked exactly the same and was I worried that I was missing something (since the plots have been changing in most of the other unit PRs). Good to know that this component needed to look the same to be correct. Thank you!

MarcelCaron-NOAA commented 1 month ago

Yes! Those plots should not change. I updated the PR instructions to include that info. Thanks!

MarcelCaron-NOAA commented 1 month ago

The following completed cleanly and output look normal: ✔️ jevs_analyses_grid2obs_plots

⏳ jevs_cam_grid2obs_plots ⏳ jevs_mesoscale_grid2obs_plots

MarcelCaron-NOAA commented 1 month ago

The following completed cleanly and output look normal: ✔️ jevs_mesoscale_grid2obs_plots

⏳ jevs_cam_grid2obs_plots

Ho-ChunHuang-NOAA commented 1 month ago

@AliciaBentley-NOAA @MarcelCaron-NOAA AQM plots use Marcel's code as the core modules. AQM uses only fbar and obar. But it would be good to sync the updated code with Marcel 's, and thank you Marcel for doing the lift.

MarcelCaron-NOAA commented 1 month ago

@malloryprow The following completed cleanly and output look normal: ✔️ jevs_cam_grid2obs_plots

👍 All of the test jobs have completed successfully

malloryprow commented 1 month ago

Thanks for your work on this @MarcelCaron-NOAA!

NOAA-EMC / EVS