Skip to content

In Noah-MP, set zero for canopy liquid and canopy ice over glacier#278

Merged
grantfirl merged 4 commits into
ufs-community:ufs/devfrom
wzzheng90:canopywater_glacier
Jun 5, 2025
Merged

In Noah-MP, set zero for canopy liquid and canopy ice over glacier#278
grantfirl merged 4 commits into
ufs-community:ufs/devfrom
wzzheng90:canopywater_glacier

Conversation

@wzzheng90
Copy link
Copy Markdown

This PR addresses the issue #272 (#272)
It updated the old PRs #275 (#275).

@wzzheng90
Copy link
Copy Markdown
Author

Added the modification for snow glacier in "module_sf_noahmp_glacier.F90" from Mike Barlage.

@wzzheng90
Copy link
Copy Markdown
Author

@barlage Re: "@wzzheng90 Thinking beyond this immediate issue, are there any other glacier outputs that make it to the final product files that also should be changed?"
I checked the surface tile files and found almost all glacier variables related to canopy are assigned with a missing value (9.99e20_kind_phys). These variables are "canopy moisture content" (2*9.99e20_kind_phys), "vegetation leaf temperature", "bulk ground surface temperature", "canopy-intercepted ice", "canopy-intercepted liquid water", "canopy air vapor pressure", "canopy air temperature", and "wetted or snowed fraction of the canopy". In the Gaussian grid surface NetCDF files, the missing variables over the glacier include "canopy water", "total evaporation of intercepted water", and "total plant transpiration". Do you think it needs to assign "0.0" or "273" (for temperature) for all?

@wzzheng90
Copy link
Copy Markdown
Author

Further updated the glacier variables related to the canopy and set all the undefined over the glacier to some values suggested by Mike Barlage.

@wzzheng90
Copy link
Copy Markdown
Author

Performed some regression tests ( with Gaussian_grid and cubed_sphere_grid, separately ) for two sensitivity runs: a) setting all the "undefined" over glacier to some values, and b) modification of the snow glacier ( "module_sf_noahmp_glacier.F90" ) plus a) changes.
The a) run has no impacts at the non-glacier grids which we expected, while the b) run indicates some small changes of simulation. Several slides (PPT) are attached.

Copy link
Copy Markdown
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

@wzzheng90 Code changes look OK. Please reinstate the pull request templates for your ufs-weather-model and fv3atm pull requests. In addition, please add your test_changes.list and regression testing log to the ufs-weather-model pull request.

@wzzheng90
Copy link
Copy Markdown
Author

@grantfirl Thanks! Yes, I am working on reinstating the pull request templates for my ufs-weather-model and fv3atm pull requests, as well as regression results.

@yangfanglin
Copy link
Copy Markdown
Collaborator

Weizhong and RhaeSun have run some RT tests, all but the rrfs_v1beta_debug_intel failed. Since RRFSv1 is not using NOAH-MP, does anyone know if this RT is needed ? We need to move this PR forward asap. We do not have the resources to debug why this particular test is failing. My recommendation is to turn off rrfs_v1beta_debug_intel anf rrfs_v1beta_intel

@grantfirl
Copy link
Copy Markdown
Collaborator

grantfirl commented May 28, 2025

Weizhong and RhaeSun have run some RT tests, all but the rrfs_v1beta_debug_intel failed. Since RRFSv1 is not using NOAH-MP, does anyone know if this RT is needed ? We need to move this PR forward asap. We do not have the resources to debug why this particular test is failing. My recommendation is to turn off rrfs_v1beta_debug_intel anf rrfs_v1beta_intel

@wzzheng90 @rhaesung @yangfanglin I think that rrfs_v1_beta_debug tests are expected to fail. If I look in tests/tests/rrfs_v1beta_debug, I see export_rrfs_v1 which triggers that function in default_vars.sh. In default_vars.sh, I see export CCPP_SUITE=FV3_RRFS_v1beta. In suite_FV3_RRFS_v1beta, I see noahmp as the LSM. Since this PR should change all NoahMP tests, I think that this is expected and there is no reason to wait to debug anything. Am I wrong?

@yangfanglin
Copy link
Copy Markdown
Collaborator

@grantfirl The test crashed, not du to change of results

46: 49 0x0000000000c94b81 esmf_gridcompmod_mp_esmf_gridcomprun_() /contrib/spack-stack/spack-stack-1.6.0/cache/build_stage/role.epic/spack-stage-esmf-8.6.0-lj3j3za5cnw5mxrzai3gv3lc6whbvnyw/spack-src/src/Superstructure/Component/src/ESMF_GridComp.F90:1903
46: 50 0x000000000042fdec MAIN__() /scratch2/NCEPDEV/land/Weizhong.Zheng/noscrub/git/CanopyWater/ufs-weather-model/driver/UFS.F90:409
46: 51 0x000000000042c0a2 main() ???:0
46: 52 0x000000000003a7e5 __libc_start_main() ???:0
46: 53 0x000000000042bfae _start() ???:0
46: =================================
46: forrtl: error (75): floating point exception
46: Image PC Routine Line Source
46: fv3.exe 000000001483390B Unknown Unknown Unknown
46: libpthread-2.28.s 0000151239DC4990 Unknown Unknown Unknown
46: fv3.exe 0000000012C6E980 noahmp_glacier_ro 2664 module_sf_noahmp_glacier.F90
46: fv3.exe 0000000012C6C203 noahmp_glacier_ro 2524 module_sf_noahmp_glacier.F90
46: fv3.exe 0000000012C2236E noahmp_glacier_ro 324 module_sf_noahmp_glacier.F90

@wzzheng90 @rhaesung please provide more details

@wzzheng90
Copy link
Copy Markdown
Author

Weizhong and RhaeSun have run some RT tests, all but the rrfs_v1beta_debug_intel failed. Since RRFSv1 is not using NOAH-MP, does anyone know if this RT is needed ? We need to move this PR forward asap. We do not have the resources to debug why this particular test is failing. My recommendation is to turn off rrfs_v1beta_debug_intel anf rrfs_v1beta_intel

@wzzheng90 @rhaesung @yangfanglin I think that rrfs_v1_beta_debug tests are expected to fail. If I look in tests/tests/rrfs_v1beta_debug, I see export_rrfs_v1 which triggers that function in default_vars.sh. In default_vars.sh, I see export CCPP_SUITE=FV3_RRFS_v1beta. In suite_FV3_RRFS_v1beta, I see noahmp as the LSM. Since this PR should change all NoahMP tests, I think that this is expected and there is no reason to wait to debug anything. Am I wrong?

Thanks Fanglin and Grant's comments. In terms of failed RRFS's runs, I found that "dzsnso(0)" can become zero and result in floating point exception:

if(sneqv > mwd) then ! 100 mm -> maximum water depth
bdsnow = snice(0) / dzsnso(0)
....
endif

BTW, There are many revisions above this line. Here just list some variables related to this issue

dzsnso: snow/soil layer thickness [m]
isnow: actual no. of snow layers
sneqv: snow water eqv. [mm]
mwd: =100mm, maximum water depth

When snow water equivalent > 100 mm, quite surprised to see that dzsnso(0) is zero? ?
Any comments/suggestions?

!to obtain equilibrium state of snow in glacier region

if(sneqv > mwd .and. isnow /= 0) then ! 100 mm -> maximum water depth
if(sneqv > mwd) then ! 100 mm -> maximum water depth
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@wzzheng90 Why remove the isnow /= 0 test? This is preventing division by zero in line 2664, which is causing the RT failure.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This was added in #120 to fix the same issue.

@wzzheng90
Copy link
Copy Markdown
Author

@grantfirl I checked Mike's modifications, and it did not work even if I kept isnow /= 0, for there are lots of changes above this line.

@grantfirl
Copy link
Copy Markdown
Collaborator

grantfirl commented May 28, 2025

OK, please add an issue in ufs-weather-model to fix these tests later if you want to disable them to get this PR in. Reference this PR/discussion so that whomever works on it in the future has this context.

@wzzheng90
Copy link
Copy Markdown
Author

OK, please add an issue in ufs-weather-model to fix these tests later if you want to disable them to get this PR in. Reference this PR/discussion so that whomever works on it in the future has this context.

@grantfirl Thanks for your suggestions. I added it in ufs-weather-model.

@FernandoAndrade-NOAA
Copy link
Copy Markdown

@grantfirl @rhaesung could you help @wzzheng90 with syncing up the branch? Thank you.

@FernandoAndrade-NOAA
Copy link
Copy Markdown

Testing on #2723 has completed, please continue with the merge process for this PR, thank you.

@grantfirl grantfirl merged commit 2574825 into ufs-community:ufs/dev Jun 5, 2025
1 of 3 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in GFS v17 Jun 5, 2025
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.

8 participants