-
Notifications
You must be signed in to change notification settings - Fork 108
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
Single step oro & fix files generation procedure for atm and ocn res combo #830
Single step oro & fix files generation procedure for atm and ocn res combo #830
Conversation
New scripts and codes were written to reduce the multi step procedure of generating coupled grids into a single step process. During the development of this new single step process, the original “orog” code was split into two parts that is sequentially activated by flags in the scripts to first generate a land mask and then use the land mask generated by the ocean merge procedure to populate the orography fields. The following information documents the effects of these code changes. Detailed results are here Five different comparisons were made (Inference)
|
I tried running 0b9a03e on Hera. I used the default settings. I got this error:
|
Thanks for catching this bug. I just fixed and tested it . The issue was incorrect paths in the ocean merge routine. I just fixed and tested the default settings with empty directories. |
To get the regression tests to work, I had to update these scripts.
Also, since the default for uniform grids will now include the ocean merge step, I want to update this regression test to run that step:
Please find all updated scripts on Orion here: |
Hi George, although the "ocn " is defined in the driver, it is not declared as a default in the /ush/fv3gfs_driver*.sh. This way the grid gen test 1 will run and produce results that fails the original only because of the different orog . I agree adding a test is the way forward. I will copy over these files and merge them in this. Thanks. |
driver_scripts/driver_grid.hera.sh
Outdated
#----------------------------------------------------------------------- | ||
|
||
export home_dir=$SLURM_SUBMIT_DIR/.. | ||
export TEMP_DIR=/scratch2/NCEPDEV/stmp1/$LOGNAME/fv3_grid.$gtype | ||
export out_dir=/scratch2/NCEPDEV/stmp1/$LOGNAME/my_grids | ||
export out_dir=/scratch2/NCEPDEV/stmp1/$LOGNAME/my_grids_ocean_fix/ | ||
#export ocean_mask_dir=/scratch1/NCEPDEV/stmp4/Sanath.Kumar/ocean_mask/CPLD_GRIDGEN/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would set ocean_mask_dir in fv3gfs_driver_grid.sh using the $home_dir variable to set the path. Then, you don't need to declare it in every machine's driver script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HI @GeorgeGayno-NOAA ,
I had to use dirname in /ush/fv3gfs_ocean_merge.sh as the fortran merge code could not handle the "../" relative paths.
ocean_mask_dir="$(dirname $(dirname
It works now. But is there a better solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try this:
Update fv3gfs_ocean_merge.sh
as follows:
cat << EOF > input.nml
&mask_nml
- ocean_mask_dir="$(dirname $(dirname $home_dir))/fix/orog/C${res}/ocean_mask/${ocn}/"
+ ocean_mask_dir="${home_dir}/fix/orog/C${res}/ocean_mask/${ocn}/"
ocnres="mx${ocn}"
lake_mask_dir="${TEMP_DIR}/C${res}/orog/"
atmres="C${res}"
@@ -22,6 +23,8 @@ EOF
and merge_lake_ocnmsk.f90
as follows
integer :: binary_lake
- character(len=120) :: flnm
+ character(len=250) :: flnm
integer :: ncid,ndims,nvars,natts,lat,lon,v1id,v2id,v3id,v4id,start(2),count(2),i,j,latid,lonid,ncid4, dims(2),tile,nodp_pt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This worked. Thanks @GeorgeGayno-NOAA
ush/fv3gfs_driver_grid.sh
Outdated
|
||
cat <<EOF > $readme_name | ||
The following # was used | ||
https://github.com/sanatcumar/UFS_UTILS/tree/single_step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
References to your github branch won't work after the merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just thought of something. The logic in ./reg_tests/get_hash.sh
is used to get the github commit hash. Could the hash be added as part of the readme file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That worked great!
How are you pointing to the new fixed directory without updating |
I linked just the orog . I updated and and testing it now |
Since the name of the
|
Pointing to the 20231027 version of the 'fix' files results in some UFS_UTILS utility programs to break. These will be fixed under #867. However, one small fix that I would like to include in this PR is: Thanks. I think this is the last change before we finally merge. |
The regression tests were run on all Tier 1 machines using d0d1e8f. Only the
Other |
Orography is set to zero when lake fraction and slmask is zero in the original codes. This causes some fractional land points near oceans to have zero orography with the inclusion of newer codes to generate atmosphere and ocean coupled grids. To avoid this error new logic was added to sorc/orog_mask_tools.fd/orog.fd/mtnlm7_oclsm.F to set orography to zero only when both lake and land frac is zero. Small differences are expected. @barlage please chime in if I have missed anything. |
@sanatcumar - I have no more comments. Unless you object, I will merge tomorrow. |
I do not have any further comments/ suggestions. Thanks George |
DESCRIPTION OF CHANGES:
Consolidated the multi-step process for generating the oro and fix files for combinations of atmospheric and ocean grid resolutions.
TESTS CONDUCTED:
grid_gen
tests passed. This result was expected. For details on thegrid_gen
tests, see below.Describe any additional tests performed.
Pasted below is a summary of the tests conducted.
New scripts and codes were written to reduce the multi step procedure of generating coupled grids into a single step process.
During the development of this new single step process, the original “orog” code was split into two parts that is sequentially activated by flags in the scripts to first generate a land mask and then use the land mask generated by the ocean merge procedure to populate the orography fields. The following information documents the effects of these code changes.
Five comparisons with baselines were made.
DEPENDENCIES:
#843 must be merged first. #854 needs to be merged first
DOCUMENTATION:
The repository was compiled on Hera with Doxygen (d0d1e8f). The
orog.fd
andocean_merge.fd
had no Doxygen warnings.ISSUE:
Fixes #825.
CONTRIBUTORS:
@GeorgeGayno-NOAA , @shansun6 , @aerorahul , @barlage