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

Merge Radmon scripts for monitoring the satellite radiance DA #162

Merged
merged 19 commits into from
Jan 18, 2024

Conversation

xyzemc
Copy link
Contributor

@xyzemc xyzemc commented Dec 12, 2023

This PR is for run the Radiance Monitor (Radmon) Package in RRFS workflow for generating the real-time the satellite radiance DA statistics information, like O-B/O-A, bias correction, data number for each instrument and each channel. The code has been merged to rrfs_utl as described in: NOAA-GSL/rrfs_utl#43, and NOAA-GSL/rrfs_utl#46.

This PR includes:
Added the rrfs_radmon directory under ./ush;
Added the required fix files for running radmon;
Modifiedexrrfs_run_gsidiag.sh scripts to run radmon, and setup some local variables that radmon scripts required;
Added the 'gzip' in exrrfs_run_gsidiag.sh to compress the diag file for saving the space, and fit the requirement of radmon scripts.

ISSUE:

…diance;

Added the required fix files for running radmon;
Added the gzip in gsi scripts to comporess the diag file for saving disk space.
fix/gsi/gdas_radmon_base.tar Outdated Show resolved Hide resolved
ush/rrfs_radmon/clean_tankdir.sh Show resolved Hide resolved
jobs/JRRFS_RUN_GSIDIAG Outdated Show resolved Hide resolved
jobs/JRRFS_RUN_GSIDIAG Outdated Show resolved Hide resolved
scripts/exrrfs_run_gsidiag.sh Outdated Show resolved Hide resolved
scripts/exrrfs_run_gsidiag.sh Outdated Show resolved Hide resolved
ush/rrfs_radmon/exnam_verfrad.sh Outdated Show resolved Hide resolved
ush/rrfs_radmon/exnam_verfrad.sh Outdated Show resolved Hide resolved
@chan-hoo
Copy link
Contributor

@xyzemc, could you open an issue for this PR?

@chan-hoo
Copy link
Contributor

@xyzemc, I found that an issue for this PR had been already issued. I'll add it to the above description.

@chan-hoo chan-hoo linked an issue Dec 13, 2023 that may be closed by this pull request
@xyzemc
Copy link
Contributor Author

xyzemc commented Dec 13, 2023

@chan-hoo Thanks for linking the issue to this PR.

xyzemc and others added 4 commits December 13, 2023 16:06
Confirmed with Ed. this file is not needed for radmon in RRFS workflow.
moved DO_RADMON="FALSE" from exrrfs_run_gsidiag.sh to config_defaults.sh
@MatthewPyle-NOAA
Copy link
Contributor

@xyzemc I'd like to get this one merged today or tomorrow if possible. A big merge that went in today has created some script conflicts that need to be resolved.

@xyzemc
Copy link
Contributor Author

xyzemc commented Dec 18, 2023

@xyzemc I'd like to get this one merged today or tomorrow if possible. A big merge that went in today has created some script conflicts that need to be resolved.

resolved all the conflicts.

Copy link
Contributor

@MatthewPyle-NOAA MatthewPyle-NOAA left a comment

Choose a reason for hiding this comment

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

Congrats on clearing out the conflicts, @xyzemc. Just one small request before approving.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be renamed to either exrrfs_verfrad.sh or radmon_verfrad.sh? Mostly just would like to get rid of the "nam" aspect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could this be renamed to either exrrfs_verfrad.sh or radmon_verfrad.sh? Mostly just would like to get rid of the "nam" aspect.

Yes. I was thinking that too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @xyzemc I'd like to run a quick test to make sure nothing breaks with these changes, but looks good to me.

Copy link
Contributor

@MatthewPyle-NOAA MatthewPyle-NOAA left a comment

Choose a reason for hiding this comment

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

Giving conditional approval, but should a DA-engineering test be run? What do you think, @chan-hoo?

@chan-hoo
Copy link
Contributor

@MatthewPyle-NOAA, sorry for my late response. Yes, I think so. We should check at least two tasks "run_analysis" and "run_gsidiag" in a DA eng test.

@MatthewPyle-NOAA
Copy link
Contributor

@chan-hoo is a DA engineering test best done on WCOSS2? Real time or retro? I've never done much with it. Thanks!

@chan-hoo
Copy link
Contributor

@MatthewPyle-NOAA, I agree. I also feel it is not good as an engineering test. I think we'll need a retro or ensemble case for wcoss2 (not real-time).

@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 2, 2024

@xyzemc, could you let us know which sample configuration we can use to test this PR?

@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 3, 2024

@xyzemc, currently the DA engineering test configuration does not work on Hera (Issue #180). Once it is available, I'll test your PR.

@xyzemc
Copy link
Contributor Author

xyzemc commented Jan 3, 2024

@xyzemc, currently the DA engineering test configuration does not work on Hera (Issue #180). Once it is available, I'll test your PR.

Thank you!

scripts/exrrfs_run_gsidiag.sh Outdated Show resolved Hide resolved
scripts/exrrfs_run_gsidiag.sh Outdated Show resolved Hide resolved

#################################################################################

export USHradmon=${USHradmon:-${USHDIR}/rrfs_radmon}
Copy link
Contributor

Choose a reason for hiding this comment

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

${USHDIR} -> ${USHdir}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not changed yet.

ush/rrfs_radmon/exrrfs_verfrad.sh Outdated Show resolved Hide resolved
@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 9, 2024

@xyzemc, the test run failed. Please resolve the issues. You can use the engineering test script ush/sample_configs/DA_eng/config.DA.retro.hera.sh.

@xyzemc
Copy link
Contributor Author

xyzemc commented Jan 9, 2024

@xyzemc, the test run failed. Please resolve the issues. You can use the engineering test script ush/sample_configs/DA_eng/config.DA.retro.hera.sh.

Will do it when Hera is back from maintenance.

@MatthewPyle-NOAA
Copy link
Contributor

@xyzemc Where does this PR stand now?

@xyzemc
Copy link
Contributor Author

xyzemc commented Jan 18, 2024

@xyzemc Where does this PR stand now?

Still working on the engineering test

@MatthewPyle-NOAA
Copy link
Contributor

@xyzemc Where does this PR stand now?

Still working on the engineering test

Okay, thanks. No hurry - just wanted to make sure I wasn't holding up the process.

@xyzemc
Copy link
Contributor Author

xyzemc commented Jan 18, 2024

Added an option in exrrfs_run_gsidiag.sh to skip the radmon job when the 'radstat' file is not existing.
Engendering test has been pasted from my end.

@chan-hoo
Copy link
Contributor

Thanks for the update. I'm testing it again now.

@chan-hoo
Copy link
Contributor

It looks working well. Approving.

@MatthewPyle-NOAA MatthewPyle-NOAA self-requested a review January 18, 2024 19:19
Copy link
Contributor

@MatthewPyle-NOAA MatthewPyle-NOAA left a comment

Choose a reason for hiding this comment

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

Reapproving as well, and will merge. Thanks for seeing this one through, @chan-hoo and @xyzemc

@MatthewPyle-NOAA MatthewPyle-NOAA merged commit acbeb01 into NOAA-EMC:dev-sci Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add radiance monitoring package to workflow
4 participants