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

DEPHY conversion #496

Merged
merged 31 commits into from
Aug 13, 2024
Merged

DEPHY conversion #496

merged 31 commits into from
Aug 13, 2024

Conversation

grantfirl
Copy link
Collaborator

@grantfirl grantfirl commented Jul 24, 2024

  • Add dephy_converter.py script that takes one argument (-n name_of_case) where name_of_case is found in scm/etc/case_config and data file found in scm/data/processed_case_input and produces a DEPHY-formatted case namelist and data file
  • This script successfully converted all previous cases except for fv3_model_point_noah that was outdated.
  • Files in scm/etc/case_config are updated
  • Files in scm/data/processed_case_input were updated in v7.0.0-beta release asset
  • Fixed small bugs in run_scm.py, scm_forcing.F90, and scm_input.F90

@grantfirl
Copy link
Collaborator Author

#481 and #463 have been combined into here.

@grantfirl
Copy link
Collaborator Author

I have updated DEPHY files for all data in data/processed_case_input ready to be made into a release asset.

@@ -21,7 +21,7 @@ for file in "${data_files[@]}"; do
mkdir -p $BASEDIR/scm/data/$file
cd $BASEDIR/scm/data/$file
echo "Retrieving $file"
wget https://github.com/NCAR/ccpp-scm/releases/download/v6.0.0/${file}.tar.gz
wget https://github.com/NCAR/ccpp-scm/releases/download/v7.0.0-beta/${file}.tar.gz
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mkavulich As soon as there is a v7.0.0-beta release on GitHub, this script is ready to go and CI should use the new files.

#height = ffc.get_height_from_pres(T_abs[:,0],levels,z_sfc)

#the following variables are not in this forcing file, but are included in other cases
soil_depth = np.zeros(4,dtype=float)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hertneky It's not showing as changed because this file is new, but I added this (needed by the dephy_converter.py script when lsm_ics = T)

snwdph = 2.e-4
tg3 = 271.35
zorl = 15.0
alvsf = 0.06
Copy link
Collaborator Author

@grantfirl grantfirl Aug 1, 2024

Choose a reason for hiding this comment

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

@hertneky Updated these (alvsf, alnsf, alvwf, alnwf, facsf, facwf). The previous values were from the GABLS3 case, but those are specific to that location. These are the values from the UFS fix files for this lat/lon. It doesn't seem to change the results though.

canopy = 0.0
vegtyp = 0
soiltyp = 0
scolor = 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hertneky Added scolor needed by lsm_ics.

shdmax = 0.0
slopetyp = 1
snoalb = 0.0
sncovr = 1.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hertneky Added sncovr needed by lsm_ics.

writefile_zorl_var.units = 'cm'
writefile_zorl_var.description = 'composite surface roughness length'

writefile_zorll_var = writefile_scalar_grp.createVariable('zorll', 'f4')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hertneky Writing out zorll, zorlw, zorli as identical to zorl -- needed by lsm_ics.

@@ -0,0 +1,590 @@
#!/usr/bin/env python
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hertneky Everything that I added to the MOSAiC_AMPS_forcing... file, I repeated here.

call NetCDF_read_var(ncid, "pa", .True., input_pres)
call NetCDF_read_var(ncid, "zh", .True., input_height)

if (lev_in_altitude) then
Copy link
Collaborator Author

@grantfirl grantfirl Aug 1, 2024

Choose a reason for hiding this comment

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

@bluefinweiwei This code change is to accommodate the COMBLE case that doesn't supply everything that a DEPHY file should have. One has a choice of what to put in the 'lev' data in the NetCDF file (according to the DEPHY file specification). This code can now handle whether the data is altitude in m or pressure levels in Pa. With this change, the other changes that you had to the scm_input.F90 file are taken care of.

else if (trim(input_surfaceForcingLSM) == 'lsm') then
end if

if (trim(input_surfaceForcingLSM) == 'lsm') then
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note to self: This is needed to not have LSM ICs be mutually exclusive to surfaceForcingT == 'ts'. For the MOSAiC cases, we have a forcing surface T and LSM IC data for the sea ice. This allows us to read in LSM IC data and force the surface temperature, like the case wants. This does not break the GABLS3 cases (tested).

@@ -2259,8 +2318,8 @@ subroutine get_case_init_DEPHY(scm_state, scm_input)
!set all individual w forcing controls to .true. until finer control is available from the input file
scm_state%force_sub_for_T = .true.
scm_state%force_sub_for_qv = .true.
scm_state%force_sub_for_u = .true.
scm_state%force_sub_for_v = .true.
!scm_state%force_sub_for_u = .true.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This can potentially be uncommented. Subsidence forcing typically isn't applied to horizontal momentum and the DEPHY standard doesn't have a control for which variables to apply w or omega forcing to. This is simply an assertion on my part.

@@ -27,6 +27,17 @@
{"case": "LASSO_2016051812", "suite": "SCM_GFS_v16"}, \
{"case": "LASSO_2016051812", "suite": "SCM_WoFS_v0"}, \
{"case": "LASSO_2016051812", "suite": "SCM_HRRR_gf"}, \
{"case": "COMBLE", "suite": "SCM_GFS_v17_p8_ugwpv1"}, \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ligiabernardet @dustinswales Added COMBLE/MOSAiC to RTs

…shorter and will make list maintenance less of a hassle if changes are needed
     - Fix calls to logging.critical() with no argument
Copy link
Collaborator

@mkavulich mkavulich left a comment

Choose a reason for hiding this comment

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

@grantfirl I have opened two PRs to your branch with proposed changes (only one of which should be merged): one of which is my "preferred" changes and the other I'd consider "essential" changes. Note that I haven't run any tests yet (since I'm not sure where to get that data) but the changes should still produce bit-for-bit identical output.

grantfirl#11 These are my preferred changes. I understand that it is invasive, but it eliminates so much code redundancy that doesn't seem necessary. Also some fixes to problems mentioned below.

grantfirl#12 These are my "essential" changes. It converts the huge multi-line regular string with arguments at the end to a multi-line f-string, which is much easier to maintain.

Both of the above PRs include two bug fixes:

  1. a fix to a bug which may require that data gets re-converted: w_d was assigned the value of w_0.
  2. Several calls to logging.critical() that included no message string as argument, which is required.

I also included "debug" as a new argument to get debug output rather than requiring manual editing of code.

scm/etc/scripts/dephy_converter.py Outdated Show resolved Hide resolved
scm/etc/scripts/dephy_converter.py Outdated Show resolved Hide resolved
scm/etc/scripts/dephy_converter.py Outdated Show resolved Hide resolved
scm/etc/scripts/dephy_converter.py Show resolved Hide resolved
scm/etc/scripts/dephy_converter.py Show resolved Hide resolved
@grantfirl grantfirl requested a review from mkavulich August 10, 2024 00:04
logging.basicConfig(format='%(levelname)s: %(message)s', level=logging.DEBUG)
else:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whoops, good catch!

Copy link
Collaborator

@mkavulich mkavulich left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes, just a few more minor suggestions that are not required for merging.

scm/etc/scripts/scm_analysis.py Outdated Show resolved Hide resolved
scm/src/scm_input.F90 Outdated Show resolved Hide resolved
scm/src/scm_input.F90 Show resolved Hide resolved
scm/src/scm_input.F90 Outdated Show resolved Hide resolved
scm/etc/scripts/scm_read_obs.py Show resolved Hide resolved
Copy link
Collaborator

@dustinswales dustinswales left a comment

Choose a reason for hiding this comment

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

@grantfirl These changes look good to me. Thanks for this, it was a big effort to convert all of the cases to DEPHY.
I had a super picky comment, but nothing to hold this up.

scm/etc/case_config/COMBLE.nml Outdated Show resolved Hide resolved
@grantfirl grantfirl merged commit 7ede232 into NCAR:main Aug 13, 2024
19 of 24 checks passed
@grantfirl
Copy link
Collaborator Author

@grantfirl
Copy link
Collaborator Author

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.

4 participants