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 the RadMon data extract source code into rrfs_util. #46

Merged
merged 6 commits into from
Nov 2, 2023

Conversation

xyzemc
Copy link
Contributor

@xyzemc xyzemc commented Oct 3, 2023

#43

The RadMon (radiance monitoring) package is used to extract data from
radiance diagnostic files to generate the binary files for plotting the statistics from radiance data assimilation, and generate tons of images for each satellite instrument and channel. It has been used in GDAS and NAM DA operational.
This pull request is for adding the RadMon source code into rrfs_util.

The data extract step from RadMon include four packs of source code:
verf_radang.fd/
verf_radbcoef.fd/
verf_radbcor.fd/
verf_radtime.fd/

All these codes have put in the directory 'radmon' under rrfs_util. The corresponding makefile has been modified and tested. Four executables will be placed into the ./bin directory as all other code did.

Copy link
Collaborator

@christopherwharrop-noaa christopherwharrop-noaa left a comment

Choose a reason for hiding this comment

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

There is a lot of duplication in this pull request. The specification of compiler flags for different compilers should ideally be done in one place for the entire repository, not duplicated verbatim in multiple places. Additionally, the introduction of multiple kinds.f90 creates unnecessary duplication, particularly since rrfs_utls already has a kinds module. Please remove duplication and make use of existing code and build specifications where possible.

@hu5970
Copy link
Contributor

hu5970 commented Oct 3, 2023

@christopherwharrop-noaa This repository is to host many small applications for RRFS. Some of those applications are isolated from others. I agree it is good to reduce the duplication. We also need to check this case by case. "kind.f90" used here may not have the same definition as the kinds module used by other. Do we have a way to keep some code used locally (like kinds.f90 only used by this application)? The same possible is also applying for compiling options.

@xyzemc
Copy link
Contributor Author

xyzemc commented Oct 3, 2023

@christopherwharrop-noaa @hu5970 I will compare the kind.f90 in this PR and the existing kind.f90 in rrfs_util to see if they are same and can avoid the duplication.

… a seperate directory.

Modified the CMake file to generate three module (.mod) in the include directory.
@hu5970
Copy link
Contributor

hu5970 commented Nov 2, 2023

@xyzemc Please remove save directory and sync with current develop branch.

Copy link
Collaborator

@christopherwharrop-noaa christopherwharrop-noaa left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to address my comments. This looks great! There are a couple commented lines in CMakeLists.txt files that should be removed, and you might also consider removing what look like (I'm not sure) commented out debugging write statements. Once the Cmake comments are removed, I'll approve.

radmon/shared/CMakeLists.txt Outdated Show resolved Hide resolved
radmon/verf_radbcoef.fd/CMakeLists.txt Outdated Show resolved Hide resolved
radmon/verf_radtime.fd/bad_chan.f90 Outdated Show resolved Hide resolved
radmon/verf_radtime.fd/bad_penalty.f90 Outdated Show resolved Hide resolved
Modified the kinds.mod to kinds_radmon.mod in share/kinds_radmon.F90.
Modified the 'use kinds' to 'use kinds_radmon'
Copy link
Collaborator

@christopherwharrop-noaa christopherwharrop-noaa left a comment

Choose a reason for hiding this comment

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

Thank you for making those last few updates.

@guoqing-noaa guoqing-noaa merged commit 23e60b4 into NOAA-GSL:develop Nov 2, 2023
1 check passed
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.

4 participants