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

Add VIIRS SCF processing capability and merge with current IMS_proc #7

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

yuanxue2870
Copy link

@yuanxue2870 yuanxue2870 commented Aug 5, 2024

Describe your changes (READY FOR REVIEW/DO NOT MERGE!)

Summarise all code changes included in PR:
1.added VIIRS processing unit;
2.merged VIIRS processing unit with IMS processing one;
3.added VIIRS super-ob percentage threshold as a parameter in the namelist file;
4.added a new VIIRS test case, and separated from previous IMS test cases.

List any associated PRs in the submodules.
This is the PR for the submodule land-SCF_proc.

Issue ticket number and link

List the git Issue that this PR addresses:
#6

Test output

Is this PR expected to pass the DA_IMS_test (ie., does it change the output)?
Yes.

Does it pass the DA_IMS_test?
Yes, it passed the IMS processing test case for both .nc and .ascii formatted.

If changes to the test results are expected, what are these changes? Provide a link to the output directory when running the test:
The new benchmark output for VIIRS can be found on Hera at: /scratch2/NCEPDEV/stmp1/Yuan.Xue/pull_request/land-SCF_proc/test/VIIRSscf.20160501.C384.mx025_oro_data.nc

Checklist before requesting a review

  • My branch being merged is up to date with the latest develop.
  • I have performed a self-review of my code by examining the differences that will be merged.
  • I have not made any unnecessary code changes / changed any default behavior.
  • My code passes the DA_IMS_test, or differences can be explained.

@yuanxue2870
Copy link
Author

Please review: @ClaraDraper-NOAA, @YoulongXia-NOAA, @tsga

Thank you!

CMakeLists.txt Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
sorc/Makefile Show resolved Hide resolved
RES=384

EXEC_DIR=../exec/bin/
RESTART_DIR=/scratch2/NCEPDEV/stmp1/Yuan.Xue/pull_request/restart_files/
Copy link
Contributor

Choose a reason for hiding this comment

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

The best way do not link your personal directory. Please consult with Clara to put them in land/DA/IMS or VIRRS directory.

Copy link
Author

Choose a reason for hiding this comment

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

This is for the test case restart files. For IMS, Clara linked to her own directory. For VIIRS, for now, I put them in my own stmp directory. Later, we can put these restart files for both test cases together.

namelist/fSCF_nml/ idim, jdim, source, otype, yyyymmddhh, jdate, fcst_path, lsm, skip_SD, & !required input
IMS_obs_path, IMS_ind_path, imsformat, imsversion, imsres, & !(optional) input needed for IMS DA
VIIRS_obs_path, VIIRS_ind_path, viirsversion, viirs_threshold !(optional) input needed for VIIRS DA

Copy link
Contributor

Choose a reason for hiding this comment

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

Although you used the same fscf.nml, but actually you put different contents. To make an identification, how to about still using fims.nml and fviirs.nml. Here namelist you can use two namelists. Although there are some redundancy, but it is clear. See If Clara has different opinion

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your comments. Per discussions with Clara in June, it is more preferred having only ONE namelist for both cases. Hence, I merged fims.nml and fviirs.nml as fSCF.nml. Hence, I will leave it as fSCF.nml.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you do so, you should use single submit job including all input into single nml file. Otherwise, it is confusing the users.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the reason you can consult with Clara. If it pushes to github, it should not use your personal directory based on my work experience with Mike for land-data-utils.

Copy link
Author

Choose a reason for hiding this comment

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

Leave the namelist as is for now per discussions with Clara on 08/08/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.

2 participants