-- Fix coastal lakes with revised code #856
Conversation
-- Link orog to /scratch1/NCEPDEV/global/glopara/fix/raw/orog/ to read MODISP lake fraction and GLDBV3 lake depth for now -- Add options to choose different lake datasets Fixes ufs-community#854
|
@ShanSun-NOAA your branch won't compile using GNU. To compile with GNU, adjust the build_all.sh script as follow: Here are the errors: |
|
@ShanSun-NOAA None of the consistency tests use the 'lake' option. Would it be a good idea to add one? https://github.com/ufs-community/UFS_UTILS/tree/develop/reg_tests/grid_gen I can help with that. |
|
The 'readthedocs' will need to be updated as well. I can help with that. https://noaa-emcufs-utils.readthedocs.io/en/latest/ufs_utils.html#lakefrac |
Co-authored-by: Ning Wang [email protected]
Fixed with 7dee97b. |
|
I am still getting one doxygen warning. The driver needs a return code. For example: |
Co-authored-by: Ning Wang [email protected]
Thanks @ShanSun-NOAA. It compiles now with no warnings. |
There was a problem hiding this comment.
Before merging, the following must be done:
- All scripts in ./driver_scripts must be updated for the new datasets.
- The 'readthedocs' must be updated for the new datasets.
- None of the regression tests in ./reg_tests/grid_gen excercise the 'lake' option. Do you want to add a test for lakes?
driver_scripts/driver_grid.hera.sh
Outdated
| @@ -106,10 +106,12 @@ export soil_type_src="bnu.v3.30s" # Soil type data. | |||
| # For Beijing Norm. Univ. data | |||
| # 1) "bnu.v3.30s" for global 30s data. | |||
|
|
|||
| export lake_data_srce=MODISP_GLDBV3 # 'GLDBV3', 'MODISP_GLOBATHY', 'MODISP_GLDBV3', and 'VIIRS_GLDBV3' | |||
There was a problem hiding this comment.
This new variable will also need to be defined in the driver scripts for WCOSS, Orion and Jet. I would also define what the four choices are as is done for the soil and vegetation data.
There was a problem hiding this comment.
lake_data_srce is now defined in all driver_scripts. It runs well on hera, jet and orion. The lakefrac generated on hera, jet and orion are identical.
There was a problem hiding this comment.
Q: The 'readthedocs' must be updated for the new datasets.
A: Could you help with updating 'readthedocs', as I don't have the permisson? Thanks!
Q: None of the regression tests in ./reg_tests/grid_gen excercise the 'lake' option. Do you want to add a test for lakes?
A: Lake option for regional is not ready at the moment.
There was a problem hiding this comment.
I can help with the 'readthedocs'.
One of the regression tests is for a C96 global uniform. That test has the lakes turned off. We can turn on the lakes for that test or add a new global test that uses lakes. We could also test multiple input lake datasets.
In the driver scripts, can you define what the lake source options are? For example, what is 'GLDBV3'? I assume 'VIIRS_GLDBV3' means VIIRS-base GLDBV3. The general user should know what to pick.
There was a problem hiding this comment.
In order to get 'readthedocs' to work, please merge the latest updates from 'develop' to your branch.
|
@ShanSun-NOAA Here is the documentation for lake code. Does anything need to be updated now that there are multiple choices of input data? https://noaa-emcufs-utils.readthedocs.io/en/latest/ufs_utils.html#lakefrac |
|
@ShanSun-NOAA - The updated regression test requires more wall clock time. Can you commit the following scripts from
|
Done at 3754527. Thanks, @ShanSun-NOAA |
|
@ShanSun-NOAA - Two things, then I can merge:
|
|
@ShanSun-NOAA can you update the latest changes from 'develop' into your branch? You should be able to do this directly from github without any command line interaction (let me know if that isn't correct George). Once you do this, @GeorgeGayno-NOAA can update 'readthedocs'. But from what I understand above, he can't help with readthdocs until this branch is updated with the latest develop. |
@ShanSun-NOAA readthedocs is broken. So you will need to merge from 'develop' for the fix. Also, I will need to know how you want to update this section for the additional GlobalLake options: |
|
I have updated the latest changes from 'develop' into my branch, directly from github. Thanks for your guidance. Regarding the update for 'readthedocs,' Ning has proposed the following. Please feel free to make any necessary edits. Your assistance with this matter is greatly appreciated. lake status code file - one of the following files: GlobalLakeStatus_MODISp.dat, GlobalLakeStatus_GLDBv3release.dat, GlobalLakeStatus_VIIRS.dat (specified by the shell variable lake_data_srce and located in ./fix/orog_raw). lake depth file - one of the following files: GlobalLakeDepth_GLDBv3release.dat, GlobalLakeDepth_GLOBathy.dat (specified by the shell variable lake_data_srce and located in ./fix/orog_raw). |
|
|
||
| IF (iargc() == 3) THEN | ||
| IF (iargc() == 5) THEN |
There was a problem hiding this comment.
add line after lake_cutoff line:
gridcell_lake = 1
There was a problem hiding this comment.
it seems like there is some inconsistency on default values; in some places lake_cutoff = 0.5, in others, lake_cutoff = 0.2; same for binary_lake, in some places it's 1 in others 0
| lake_frac(i) = 0. | ||
| endif | ||
|
|
||
| if (lake_frac(i) < lake_cutoff) lake_frac(i)=0. |
There was a problem hiding this comment.
add this here:
if (lake_frac(i) >= lake_cutoff .and. gridcell_lake == 1) lake_frac(i)=1.0
|
@ShanSun-NOAA can you check what I proposed above? I think we should be able to support both lake methods, i.e.,
that should give us more flexibility for having both fractional lakes and gridcell lakes |
|
Done - thanks for reminding me. -Shan
…On Wed, Nov 1, 2023 at 1:10 PM GeorgeGayno-NOAA ***@***.***> wrote:
@ShanSun-NOAA <https://github.com/ShanSun-NOAA> - make some updates to
your branch. Be sure to pull them to your local machine.
—
Reply to this email directly, view it on GitHub
<#856 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALORMVX2WIXIML3FY6VKLF3YCKNBLAVCNFSM6AAAAAA5ND4VL6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBZGUYDKMBUGU>
.
You are receiving this because you commented.
|
|
Good idea. Coming soon. -Shan
…On Wed, Nov 1, 2023 at 1:25 PM Michael Barlage ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In driver_scripts/driver_grid.hera.sh
<#856 (comment)>
:
> if [ $gtype = uniform ]; then
export res=96
- export add_lake=false # Add lake frac and depth to orography data.
- export lake_cutoff=0.20 # lake frac < lake_cutoff ignored when add_lake=T
+ export add_lake=true # Add lake frac and depth to orography data.
+ export lake_cutoff=0.50 # lake frac < lake_cutoff ignored when add_lake=T
or "# 1:only output full grid cell lakes; 0: output fractional lakes"
—
Reply to this email directly, view it on GitHub
<#856 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALORMVQREJGULZE4DOJ52S3YCKO23AVCNFSM6AAAAAA5ND4VL6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBYHA4DEMZZGU>
.
You are receiving this because you commented.
|
@ShanSun-NOAA @NingWang325 Don't we still have the option to use GLDBv2 by using GlobalLakeStatus.dat and GlobalLakeDepth.dat ? |
|
With v3, Ning has recommended renaming the file from 'GlobalLakeDepth.dat' to 'GlobalLakeDepth_GLDBv2release.dat' to ensure accuracy. As v3 is a bug-fix version of v2, we have removed v2 from the input list to discourage users from selecting it. However, v2 remains available as an option. |
|
Let's deal with the v2 issue when we convert to netcdf in the future. |
…'1 > lake_frac >= lake_cutoff' to have lake_frac= 1 or 0;
|
'binary_lake' is added as a namelist. When binary_lake=1 or 0, grid points with '1 > lake_frac >= lake_cutoff' to have lake_frac= 1 or 0. |
driver_scripts/driver_grid.hera.sh
Outdated
| export lake_cutoff=0.20 # lake frac < lake_cutoff ignored when add_lake=T | ||
| export add_lake=true # Add lake frac and depth to orography data. | ||
| export lake_cutoff=0.50 # ignore lake_frac < lake_cutoff when add_lake=T | ||
| export binary_lake=1 # return '1 > lake_frac >= lake_cutoff' as 1/0 when binary_lake=1/0 |
There was a problem hiding this comment.
this is a little confusing to me; I don't understand the need for "/0"
There was a problem hiding this comment.
when binary_lake=1, return lake_frac = 1 when lake_frac >= lake_cutoff
|
@barlage Thanks for your prompt action and detailed review. I have modified it accordingly. |
|
@barlage, have all your comments been addressed? |
There was a problem hiding this comment.
@ShanSun-NOAA @GeorgeGayno-NOAA looks good to me, thanks for making all the changes Shan
I will do a few final checks, then merge (sometime tomorrow). |
|
@ShanSun-NOAA The branch would not compile with GNU, so I made a minor update. See 58e25cd. |
|
@GeorgeGayno-NOAA Thanks for your help with GNU. |
|
All tests required before merging have passed. @shansun6, @barlage and @sanatcumar - If there are no further changes, I will merge to develop this afternoon. |
|
@sanatcumar can you confirm that the tests with @ShanSun-NOAA 's branch are working as expected? |
|
|
@sanatcumar Yes, they are doing the same thing, except binary_lake is done in lakefrac.F90 when lake frac is first created, and int_lake is done at the post-processing step. Only one of the two methods is necessary. |
As expected locations where 0 > lake_frac < 0.5 in [1] are 0 in [2] & locations where lake_frac >= 0.5 in [1] are 1 in [2]. Working as expected. |
DESCRIPTION OF CHANGES:
Link orog to
/scratch1/NCEPDEV/global/glopara/fix/raw/orog/to read MODISP lake fraction and GLDBV3 lake depth for now.Add options to choose different lake datasets
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.
The C96 uniform grid regression test failed. This is expected because the lake option was turned on for that test. All other grid regression tests passed as expected.
DEPENDENCIES:
None.
DOCUMENTATION:
Successfully compiled with 'doxygen' (no warnings with lake code) using 3e2808d on Hera.
Update 'readthedocs' for the new input datasets: https://shan.readthedocs.io/en/latest/ufs_utils.html#lakefrac
ISSUE:
Fixes issue mentioned in #854.
CONTRIBUTORS (optional):
Co-authored-by: Ning Wang [email protected]