UFS_UTILS icon indicating copy to clipboard operation
UFS_UTILS copied to clipboard

Add capability to read soil increments on the cubed-sphere tiles and enhance soil ice adjustment due to soil temperature increments

Open yuanxue2870 opened this issue 1 year ago • 22 comments

DESCRIPTION OF CHANGES:

  1. add capability to read soil (i.e., both soil temp and soil moisture) increments from jedi-based cubed-sphere tiles directly
  2. add capability to write out increments on the cubed-sphere grid from interpolating gsi-based soil increments on the Gaussian grid
  3. revise calculate_landinc_mask to make it more stable
  4. add noahmp-related parameters for soil ice adjustment after ingesting soil temp increments
  5. separate gsi based interpolation routine from its original add-increment routine
  6. combine jedi-based and gsi-based soil increments ingest and adjustment capability
  7. revise current soil ice adjustment routine based on the analysis of a thorough synthetic DA experiments conducted separately
  8. append _JEDI to the original DO_SNO_INC option for snow increment to be consistent with newly-added JEDI-based soil increment option of DO_SOI_INC_JEDI
  9. add a namelist option for DO_SOI_INC_GSI and remote GSI_SOI_FILE option in the namelist
  10. add a namelist option for lsoil_incr and set it to 3 as the default value
  11. add a unit test (ftst_read_increments) to read jedi-based soil increments on the cubed-sphere grid. This unit test requires input data, these input data will be housed elsewhere, other than Github. George is coordinating with Kate to host the test data online
  12. revise the unit test (ftst_land_increments) to ingest soil temperature increments and adjust soil ice content using newly-revised scheme
  13. add a regression test (C192.jedi_lndincsoilnoahmp.sh) to ingest jedi-based soil increments and apply adjustment. This newly-added regression test requires the same input data from the unit test. The original regression test#2 was renamed from C192.lndincsoilnoahmp.sh to C192.gsi_lndincsoilnoahmp.sh

TESTS CONDUCTED:

If there are changes to the build or source code, the tests below must be conducted. Contact a repository manager if you need assistance.

All my code edits should only affect global_cycle related unit tests and regression tests:

  • [x] Compile branch on all Tier 1 machines using Intel. Done using e3a9149.
  • [x] Compile branch on Hera using GNU. Done using e3a9149.
  • [x] Compile branch in 'Debug' mode on WCOSS2. Done using e3a9149.
  • [x] Run unit tests locally on any Tier 1 machine. Done on Hera using e3a9149.
  • [x] Run relevant consistency tests locally on all Tier 1 machine. Done using e3a9149. global_cycle tests 1, 3, and 4 passed as expected. Test 2 failed as expected. Test 5 is new.

REFERENCES:

This PR incorporates all reviews/feedback from PR#871 (https://github.com/ufs-community/UFS_UTILS/pull/871))

ISSUE:

Fixes issue mentioned in Issue#872 (https://github.com/ufs-community/UFS_UTILS/issues/872)

NOTES:

  1. Without the apply_land_da_adjustments_soil routine, C192.jedi_lndincsoilnoahmp.sh and C192.gsi_lndincsoilnoahmp.sh yields identical results. With the apply_land_da_adjustments_soil routine, C192.jedi_lndincsoilnoahmp.sh and C192.gsi_lndincsoilnoahmp.sh yields slightly different results on tile3 because GSI option checks nearby soil and no updates will be taking place for GSI grid with no soil nearby. However, JEDI option does not check nearby soil.
  2. Baseline output are required to change for regression test#2 and regression test#5.

yuanxue2870 avatar Feb 10 '24 15:02 yuanxue2870

Please re-review, @GeorgeGayno-NOAA and @ClaraDraper-NOAA, Thank you for your help!

yuanxue2870 avatar Feb 12 '24 14:02 yuanxue2870

I placed the input data - soil_xainc.00? - for one of your regression tests under the official test directory. You can now update global_cycle_driver.sh as follows:

--- a/ush/global_cycle_driver.sh
+++ b/ush/global_cycle_driver.sh
@@ -101,7 +101,7 @@ for n in $(seq 1 $ntiles); do
   fi

   if [[ "$DO_SOI_INC_JEDI" == ".true." ]] ; then
-        ln -fs $HOMEgfs/tests/global_cycle/data/soil_xainc.00$n $DATA/.
+        ln -fs $COMIN/soil_xainc.00$n $DATA/.
   fi
 done

GeorgeGayno-NOAA avatar Feb 22 '24 14:02 GeorgeGayno-NOAA

@yuanxue2870 The latest merge to develop (#899) will cause some conflicts with your branch. I can help fix them.

GeorgeGayno-NOAA avatar Feb 22 '24 14:02 GeorgeGayno-NOAA

@yuanxue2870 The latest merge to develop (#899) will cause some conflicts with your branch. I can help fix them.

Thanks for the heads up, and thanks for your help in advance!

yuanxue2870 avatar Feb 22 '24 15:02 yuanxue2870

I placed the input data - soil_xainc.00? - for one of your regression tests under the official test directory. You can now update global_cycle_driver.sh as follows:

--- a/ush/global_cycle_driver.sh
+++ b/ush/global_cycle_driver.sh
@@ -101,7 +101,7 @@ for n in $(seq 1 $ntiles); do
   fi

   if [[ "$DO_SOI_INC_JEDI" == ".true." ]] ; then
-        ln -fs $HOMEgfs/tests/global_cycle/data/soil_xainc.00$n $DATA/.
+        ln -fs $COMIN/soil_xainc.00$n $DATA/.
   fi
 done

Done. Thanks!

yuanxue2870 avatar Feb 22 '24 16:02 yuanxue2870

My wifi cutout, and I thought I'd lost my review and started again - but now I think it's still there. Ignore this for now, and give me a minute to clean it up.

I think I cleaned everything up. Thanks @yuanxue2870 for all your hard work on this. It's coming together!

ClaraDraper-NOAA avatar Feb 22 '24 22:02 ClaraDraper-NOAA

One more thought. Instead of using a single routine to interpolate (if needed), then add the increments - maybe it'd be better to have one routine for interpolating (if needed), then one for adding.

ClaraDraper-NOAA avatar Feb 22 '24 22:02 ClaraDraper-NOAA

I've had a look at all of it, except the apply_increments file, as there are too many changes for git to be helpful. I'll check it out now, and compare them.

So far, my main comments are:

  1. This is a very large PR, which makes it very difficult to review.
  2. I'm not clear that your "JEDI" soil increment files are actually from JEDI (as opposed to being increment files you created on the native model grid). If they're not from JEDI, we'll need to create some, and check that this code works with them. I can help with that.

Feel free to break up this work into multiple pull requests.

GeorgeGayno-NOAA avatar Feb 23 '24 12:02 GeorgeGayno-NOAA

One more thought. Instead of using a single routine to interpolate (if needed), then add the increments - maybe it'd be better to have one routine for interpolating (if needed), then one for adding.

Will discuss this issue via a follow-up meeting with Clara on 03/05/2024.

yuanxue2870 avatar Feb 26 '24 16:02 yuanxue2870

Currently, awaiting real-JEDI generated soil increments files from Cory/Clara based on conversations with Mike on 03/07. Based on the email communications with Cory on 03/21, real-JEDI generated soil increments will be provided on the C192 grid. While I am on hold, I am taking this PR offline and once I finish testing with the new increment files. I will put the PR back online and ping all reviewers to review. Thank you!

yuanxue2870 avatar Mar 07 '24 19:03 yuanxue2870

Currently, awaiting real-JEDI generated soil increments files from Cory/Clara based on conversations with Mike on 03/07. Based on the email communications with Cory on 03/21, real-JEDI generated soil increments will be provided on the C192 grid. While I am on hold, I am taking this PR offline and once I finish testing with the new increment files. I will put the PR back online and ping all reviewers to review. Thank you!

Thanks Cory for providing me with the real (mimicked) JEDI soil increment files. Now all tests are done. Please re-review: @GeorgeGayno-NOAA, @ClaraDraper-NOAA, @barlage.

@GeorgeGayno-NOAA: the input files used in the unit test ftst_read_increments and regression test#5 are changed. Would you please replace the test data files of soil_xainc.00N with soil_sfcincr_jedi.00N files located within /scratch2/NCEPDEV/stmp1/Yuan.Xue/pull_request/ on Hera? Thank you!

yuanxue2870 avatar Apr 04 '24 19:04 yuanxue2870

Currently, awaiting real-JEDI generated soil increments files from Cory/Clara based on conversations with Mike on 03/07. Based on the email communications with Cory on 03/21, real-JEDI generated soil increments will be provided on the C192 grid. While I am on hold, I am taking this PR offline and once I finish testing with the new increment files. I will put the PR back online and ping all reviewers to review. Thank you!

Thanks Cory for providing me with the real (mimicked) JEDI soil increment files. Now all tests are done. Please re-review: @GeorgeGayno-NOAA, @ClaraDraper-NOAA, @barlage.

@GeorgeGayno-NOAA: the input files used in the unit test ftst_read_increments and regression test#5 are changed. Would you please replace the test data files of soil_xainc.00N with soil_sfcincr_jedi.00N files located within /scratch2/NCEPDEV/stmp1/Yuan.Xue/pull_request/ on Hera? Thank you!

@KateFriedman-NOAA - you had hosted the soil_xainc.00N files here: https://ftp.emc.ncep.noaa.gov/static_files/public/UFS/ufs_utils/unit_tests/global_cycle/

Could you please replace them with the soil_sfcincr_jedi.00N files?

GeorgeGayno-NOAA avatar Apr 05 '24 20:04 GeorgeGayno-NOAA

Could you please replace them with the soil_sfcincr_jedi.00N files?

@GeorgeGayno-NOAA Done:

[emc.glopara@vm-cprk-emcrzdm03 global_cycle]$ pwd
/home/ftp/emc/static_files/public/UFS/ufs_utils/unit_tests/global_cycle
[emc.glopara@vm-cprk-emcrzdm03 global_cycle]$ ll
total 10464
-rw-r--r--. 1 emc.glopara emc 1776316 Apr  1 17:05 soil_sfcincr_jedi.001
-rw-r--r--. 1 emc.glopara emc 1776316 Apr  1 17:05 soil_sfcincr_jedi.002
-rw-r--r--. 1 emc.glopara emc 1776316 Apr  1 17:05 soil_sfcincr_jedi.003
-rw-r--r--. 1 emc.glopara emc 1776316 Apr  1 17:06 soil_sfcincr_jedi.004
-rw-r--r--. 1 emc.glopara emc 1776316 Apr  1 17:06 soil_sfcincr_jedi.005
-rw-r--r--. 1 emc.glopara emc 1776316 Apr  1 17:06 soil_sfcincr_jedi.006

KateFriedman-NOAA avatar Apr 08 '24 16:04 KateFriedman-NOAA

@yuanxue2870 I was able to successfully run the unit tests with the new files Kate hosted. I had to make these modifications:

--- a/tests/global_cycle/CMakeLists.txt
+++ b/tests/global_cycle/CMakeLists.txt
@@ -5,12 +5,12 @@

 set(CYCLE_URL "https://ftp.emc.ncep.noaa.gov/static_files/public/UFS/ufs_utils/unit_tests/global_cycle")

-set(FILE1 "soil_xainc.001")
-set(FILE2 "soil_xainc.002")
-set(FILE3 "soil_xainc.003")
-set(FILE4 "soil_xainc.004")
-set(FILE5 "soil_xainc.005")
-set(FILE6 "soil_xainc.006")
+set(FILE1 "soil_sfcincr_jedi.001")
+set(FILE2 "soil_sfcincr_jedi.002")
+set(FILE3 "soil_sfcincr_jedi.003")
+set(FILE4 "soil_sfcincr_jedi.004")
+set(FILE5 "soil_sfcincr_jedi.005")
+set(FILE6 "soil_sfcincr_jedi.006")
--- a/sorc/global_cycle.fd/cycle.f90
+++ b/sorc/global_cycle.fd/cycle.f90
@@ -782,7 +782,7 @@ ENDIF
        !--------------------------------------------------------------------------------
        ! make sure incr. files exist
         WRITE(RANKCH, '(I3.3)') (MYRANK+1)
-        JEDI_SOI_FILE = "soil_xainc." //  RANKCH
+        JEDI_SOI_FILE = "soil_sfcincr_jedi." //  RANKCH

         INQUIRE(FILE=trim(JEDI_SOI_FILE), EXIST=file_exists)
--- a/sorc/global_cycle.fd/read_write_data.f90
+++ b/sorc/global_cycle.fd/read_write_data.f90
@@ -1149,7 +1149,7 @@ MODULE READ_WRITE_DATA
  IF ((INC_FILE) .and. (DO_SNO_INC_JEDI)) THEN
         FNBGSI = "./snow_xainc." // RANKCH
  ELSEIF ((INC_FILE) .and. (DO_SOI_INC_JEDI)) THEN
-        FNBGSI = "./soil_xainc." // RANKCH
+        FNBGSI = "./soil_sfcincr_jedi." // RANKCH
  ELSE
         FNBGSI = "./fnbgsi." // RANKCH
  ENDIF

GeorgeGayno-NOAA avatar Apr 09 '24 15:04 GeorgeGayno-NOAA

@yuanxue2870 I was able to successfully run the unit tests with the new files Kate hosted. I had to make these modifications:

--- a/tests/global_cycle/CMakeLists.txt
+++ b/tests/global_cycle/CMakeLists.txt
@@ -5,12 +5,12 @@

 set(CYCLE_URL "https://ftp.emc.ncep.noaa.gov/static_files/public/UFS/ufs_utils/unit_tests/global_cycle")

-set(FILE1 "soil_xainc.001")
-set(FILE2 "soil_xainc.002")
-set(FILE3 "soil_xainc.003")
-set(FILE4 "soil_xainc.004")
-set(FILE5 "soil_xainc.005")
-set(FILE6 "soil_xainc.006")
+set(FILE1 "soil_sfcincr_jedi.001")
+set(FILE2 "soil_sfcincr_jedi.002")
+set(FILE3 "soil_sfcincr_jedi.003")
+set(FILE4 "soil_sfcincr_jedi.004")
+set(FILE5 "soil_sfcincr_jedi.005")
+set(FILE6 "soil_sfcincr_jedi.006")
--- a/sorc/global_cycle.fd/cycle.f90
+++ b/sorc/global_cycle.fd/cycle.f90
@@ -782,7 +782,7 @@ ENDIF
        !--------------------------------------------------------------------------------
        ! make sure incr. files exist
         WRITE(RANKCH, '(I3.3)') (MYRANK+1)
-        JEDI_SOI_FILE = "soil_xainc." //  RANKCH
+        JEDI_SOI_FILE = "soil_sfcincr_jedi." //  RANKCH

         INQUIRE(FILE=trim(JEDI_SOI_FILE), EXIST=file_exists)
--- a/sorc/global_cycle.fd/read_write_data.f90
+++ b/sorc/global_cycle.fd/read_write_data.f90
@@ -1149,7 +1149,7 @@ MODULE READ_WRITE_DATA
  IF ((INC_FILE) .and. (DO_SNO_INC_JEDI)) THEN
         FNBGSI = "./snow_xainc." // RANKCH
  ELSEIF ((INC_FILE) .and. (DO_SOI_INC_JEDI)) THEN
-        FNBGSI = "./soil_xainc." // RANKCH
+        FNBGSI = "./soil_sfcincr_jedi." // RANKCH
  ELSE
         FNBGSI = "./fnbgsi." // RANKCH
  ENDIF

Thanks. As Kate helps us hosting the increment files, I have now made edits to the unit test file reader and the ush/global_cycle_driver file to link directly from the public host. Your solution is also viable, but if we modify the filename in the main read_write.f90 as such, it will not be consistent with snow JEDI increment files then. I guess one of the main purposes of this PR is to make soil and snow related names/routines consistent. Therefore, I would like to retain the filename as "soil_xainc" since it is consistent with "snow_xainc". Please feel free to let me know if there are any further concerns. I am happy to adjust if needed.

yuanxue2870 avatar Apr 09 '24 16:04 yuanxue2870

@yuanxue2870 I was able to successfully run the unit tests with the new files Kate hosted. I had to make these modifications:

--- a/tests/global_cycle/CMakeLists.txt
+++ b/tests/global_cycle/CMakeLists.txt
@@ -5,12 +5,12 @@

 set(CYCLE_URL "https://ftp.emc.ncep.noaa.gov/static_files/public/UFS/ufs_utils/unit_tests/global_cycle")

-set(FILE1 "soil_xainc.001")
-set(FILE2 "soil_xainc.002")
-set(FILE3 "soil_xainc.003")
-set(FILE4 "soil_xainc.004")
-set(FILE5 "soil_xainc.005")
-set(FILE6 "soil_xainc.006")
+set(FILE1 "soil_sfcincr_jedi.001")
+set(FILE2 "soil_sfcincr_jedi.002")
+set(FILE3 "soil_sfcincr_jedi.003")
+set(FILE4 "soil_sfcincr_jedi.004")
+set(FILE5 "soil_sfcincr_jedi.005")
+set(FILE6 "soil_sfcincr_jedi.006")
--- a/sorc/global_cycle.fd/cycle.f90
+++ b/sorc/global_cycle.fd/cycle.f90
@@ -782,7 +782,7 @@ ENDIF
        !--------------------------------------------------------------------------------
        ! make sure incr. files exist
         WRITE(RANKCH, '(I3.3)') (MYRANK+1)
-        JEDI_SOI_FILE = "soil_xainc." //  RANKCH
+        JEDI_SOI_FILE = "soil_sfcincr_jedi." //  RANKCH

         INQUIRE(FILE=trim(JEDI_SOI_FILE), EXIST=file_exists)
--- a/sorc/global_cycle.fd/read_write_data.f90
+++ b/sorc/global_cycle.fd/read_write_data.f90
@@ -1149,7 +1149,7 @@ MODULE READ_WRITE_DATA
  IF ((INC_FILE) .and. (DO_SNO_INC_JEDI)) THEN
         FNBGSI = "./snow_xainc." // RANKCH
  ELSEIF ((INC_FILE) .and. (DO_SOI_INC_JEDI)) THEN
-        FNBGSI = "./soil_xainc." // RANKCH
+        FNBGSI = "./soil_sfcincr_jedi." // RANKCH
  ELSE
         FNBGSI = "./fnbgsi." // RANKCH
  ENDIF

Thanks. As Kate helps us hosting the increment files, I have now made edits to the unit test file reader and the ush/global_cycle_driver file to link directly from the public host. Your solution is also viable, but if we modify the filename in the main read_write.f90 as such, it will not be consistent with snow JEDI increment files then. I guess one of the main purposes of this PR is to make soil and snow related names/routines consistent. Therefore, I would like to retain the filename as "soil_xainc" since it is consistent with "snow_xainc". Please feel free to let me know if there are any further concerns. I am happy to adjust if needed.

Your changes at 28a8cef are fine.

GeorgeGayno-NOAA avatar Apr 09 '24 17:04 GeorgeGayno-NOAA

@ClaraDraper-NOAA and @barlage - I would like to merge this. Do you have any further comments?

@yuanxue2870 - please merge the latest updates from 'develop' to your branch.

GeorgeGayno-NOAA avatar Apr 19 '24 12:04 GeorgeGayno-NOAA

@yuanxue2870 - I will need updated baseline files for regression tests 2 and 5. I am currently using:

  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.gsi_lndincsoilnoahmp.1234
  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.jedi_lndincsoilnoahmp.1234

GeorgeGayno-NOAA avatar Apr 19 '24 14:04 GeorgeGayno-NOAA

@ClaraDraper-NOAA and @barlage - I would like to merge this. Do you have any further comments?

@yuanxue2870 - please merge the latest updates from 'develop' to your branch.

Done. Merged the latest updates from develop now.

yuanxue2870 avatar Apr 19 '24 15:04 yuanxue2870

@yuanxue2870 - I will need updated baseline files for regression tests 2 and 5. I am currently using:

  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.gsi_lndincsoilnoahmp.1234
  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.jedi_lndincsoilnoahmp.1234

My regression tests output are located on Hera at /scratch2/NCEPDEV/stmp1/Yuan.Xue/reg-tests/global-cycle/test2 for c192.gsi_lndinc and at /scratch2/NCEPDEV/stmp1/Yuan.Xue/reg-tests/global-cycle/test5 for c192.jedi_lndinc, respectively.

yuanxue2870 avatar Apr 19 '24 15:04 yuanxue2870

@yuanxue2870 - I will need updated baseline files for regression tests 2 and 5. I am currently using:

  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.gsi_lndincsoilnoahmp.1234
  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.jedi_lndincsoilnoahmp.1234

My regression tests output are located on Hera at /scratch2/NCEPDEV/stmp1/Yuan.Xue/reg-tests/global-cycle/test2 for c192.gsi_lndinc and at /scratch2/NCEPDEV/stmp1/Yuan.Xue/reg-tests/global-cycle/test5 for c192.jedi_lndinc, respectively.

For test2, the 'gaussian_interp.00?' files are outputs, correct? If so, the regression test should compare them to the baseline. And the regression test script logic will need to be updated.

GeorgeGayno-NOAA avatar Apr 19 '24 17:04 GeorgeGayno-NOAA

@yuanxue2870 - I will need updated baseline files for regression tests 2 and 5. I am currently using:

  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.gsi_lndincsoilnoahmp.1234
  • /scratch1/NCEPDEV/nems/role.ufsutils/ufs_utils/reg_tests/global_cycle/baseline_data/c192.jedi_lndincsoilnoahmp.1234

My regression tests output are located on Hera at /scratch2/NCEPDEV/stmp1/Yuan.Xue/reg-tests/global-cycle/test2 for c192.gsi_lndinc and at /scratch2/NCEPDEV/stmp1/Yuan.Xue/reg-tests/global-cycle/test5 for c192.jedi_lndinc, respectively.

For test2, the 'gaussian_interp.00?' files are outputs, correct? If so, the regression test should compare them to the baseline. And the regression test script logic will need to be updated.

Added. Done.

yuanxue2870 avatar Apr 19 '24 18:04 yuanxue2870

@yuanxue2870 - looks good. Will merge.

GeorgeGayno-NOAA avatar May 06 '24 20:05 GeorgeGayno-NOAA