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

Merge HWRF version of moninedmf into GFS version #18

Closed
wants to merge 4 commits into from

Conversation

grantfirl
Copy link
Collaborator

@grantfirl grantfirl commented Dec 10, 2019

add hurricane-specific variables to GFS_typedefs.F90 for hurricane option in moninedmf

Note: To invoke the HWRF modifications to the hybrid EDMF scheme, one needs to set the following namelist variable in gfs_physics_nml:

hurr_pbl = .true.

@grantfirl
Copy link
Collaborator Author

grantfirl commented Dec 10, 2019

@grantfirl
Copy link
Collaborator Author

@climbfuji @ligiabernardet Is there a procedure for testing HWRF components as they come into the CCPP? The standard RTs (that use the hybrid EDMF PBL scheme) should be good to test whether the HWRF modifications to moninedmf.f are confined and not active by default. I'm guessing that we'll want to at least run the FV3 using some case with the HWRF PBL modifications active (hurr_pbl = .true.) though.

@ligiabernardet
Copy link
Collaborator

@mzhangw has been testing the HWRF physics (such as F-A mp) as a one-off replacement for the corresponding physics in the GFS operational suite. The criterium for "pass" is that the meteorological results need to look "reasonable". I would recommend a multi-day forecast for at least one set of initial conditions.

@climbfuji
Copy link
Collaborator

Please add a new regression test to rt_ccpp_hafs.conf as well. Thanks!

@grantfirl
Copy link
Collaborator Author

@mzhangw has been testing the HWRF physics (such as F-A mp) as a one-off replacement for the corresponding physics in the GFS operational suite. The criterium for "pass" is that the meteorological results need to look "reasonable". I would recommend a multi-day forecast for at least one set of initial conditions.

Could I just copy the F-A test, but use the SDF and namelist for HWRF PBL? Otherwise, I'll probably need some guidance on how to do this.

@climbfuji
Copy link
Collaborator

@mzhangw has been testing the HWRF physics (such as F-A mp) as a one-off replacement for the corresponding physics in the GFS operational suite. The criterium for "pass" is that the meteorological results need to look "reasonable". I would recommend a multi-day forecast for at least one set of initial conditions.

Could I just copy the F-A test, but use the SDF and namelist for HWRF PBL? Otherwise, I'll probably need some guidance on how to do this.

I think it is pretty straightforward to copy an existing regression test and modify it for your needs, then add the entries in rt_ccpp_hafs.conf (again via copy&paste+modify).

@grantfirl
Copy link
Collaborator Author

@climbfuji Other than running the new test in rt_ccpp_hafs.conf (basically just to run the new configuration and compare against the existing HAFS configuration with F-A), which other rt*.confs would be appropriate to run with these changes? rt_full.conf to make sure that the changes don't break anything?

@climbfuji
Copy link
Collaborator

@climbfuji Other than running the new test in rt_ccpp_hafs.conf (basically just to run the new configuration and compare against the existing HAFS configuration with F-A), which other rt*.confs would be appropriate to run with these changes? rt_full.conf to make sure that the changes don't break anything?

Thanks for asking. Yes, I think that running rt.conf would be good.

climbfuji pushed a commit that referenced this pull request Feb 3, 2020
* fv3atm issue #37: fix the real(8) lat/lon in netcdf file
* fv3atm #35: Reducing background vertical diffusivities in the inversion layers
* fv3atm #24: bug in gfsphysics/physics/moninedmf_hafs.f
* fv3atm #18: Optimize netcdf write component and bugfix for post and samfdeepcnv.f
* set (0-1) bounds for ficein_cpl
* remove cache_size due to lower netcdf verion 4.5.1 on mars
* Change ice falling to 0.9 in gfsphysics/physics/gfdl_cloud_microphys.F90
@grantfirl
Copy link
Collaborator Author

@climbfuji
I ran this and associated PRs through rt.conf. All prod mode tests that use hedmf as the PBL scheme are failing (yet all REPRO tests using the same scheme pass). Since they pass in REPRO mode, I'm guessing that the code is "correct" but there is something in the changes that PROD mode is doing differently. Before I dig in to try to debug this, I thought it would be more efficient if you could have a look to see if anything in your REPRO vs. PROD expertise catches immediately, especially in NCAR/ccpp-physics#395. There really isn't that much code that has changed that is outside of the "hurr_pbl = T" sections. My first guess is that it has to do with constants through the arg list and/or "parameter" variables.

@climbfuji
Copy link
Collaborator

Hmm. Probably the same issue as for Man.

@mzhangw
Copy link
Collaborator

mzhangw commented Mar 25, 2020 via email

@grantfirl
Copy link
Collaborator Author

In order to "fix" the PROD mode failures, I kept the physical constants going through the physcons module. Something about sending grav and/or cp through the argument list and defining them as variables instead of named (parameter) constants together with the PROD compilation flags is somehow introducing differences. I don't want to keep spending time debugging this, and in fact, I remember running into this same issue the first time moninedmf.f was made CCPP-compliant (and is the reason why constants are still going through the physcons module). In order to send physical constants through the argument list, I guess we'll need to 1) modify the writing of the caps to recognize which variables are constants and preserve their constant status when they get passed into schemes or 2) put the change in whenever the PROD baselines gets updated in the future?

@climbfuji
Copy link
Collaborator

I am not sure if there is a way to accomplish (1). Maybe the "protected" attribute. On the other hand, Steve's work on cap_gen.py is going towards creating a ccpp module of constants that are collected from the host model via their standard names, and make these available in the modules (i.e. not pass them through the argument list). I don't know the details, though.

For the moment, I think it is fine to do what you did. We can make a one-time update of the regression test baseline where we fix nothing else but constants if this is still required at that time.

@grantfirl
Copy link
Collaborator Author

Here are the regression test logs (all pass except the new test, which doesn't have a baseline).

rt_full.log
rt_ccpp_dtc_create.log
rt_ccpp_dtc_verify.log

There was a time-out on the fv3_csawmg3shoc_127 test in the rt_ccpp_dtc.conf create/verify test. I ran these separately after the fact, and they passed.

rt_ccpp_dtc_create_redo.log
rt_ccpp_dtc_verify_redo.log

@climbfuji @mzhangw @ligiabernardet
This would be ready to merge into dtc/develop, but we should change the target to mzhangw fork, right? Which branch?

@mzhangw
Copy link
Collaborator

mzhangw commented Mar 30, 2020 via email

@mzhangw
Copy link
Collaborator

mzhangw commented Mar 30, 2020 via email

@grantfirl
Copy link
Collaborator Author

Is it the associated PR? NCAR/ccpp-physics#395

On Mon, Mar 30, 2020 at 1:08 PM M Zhang @.> wrote: I am creating a HWRF branch and would fetch this PR. On Mon, Mar 30, 2020 at 10:55 AM grantfirl @.> wrote: > Here are the regression test logs (all pass except the new test, which > doesn't have a baseline). > > rt_full.log https://github.com/NCAR/fv3atm/files/4404210/rt_full.log > rt_ccpp_dtc_create.log > https://github.com/NCAR/fv3atm/files/4404212/rt_ccpp_dtc_create.log > rt_ccpp_dtc_verify.log > https://github.com/NCAR/fv3atm/files/4404213/rt_ccpp_dtc_verify.log > > There was a time-out on the fv3_csawmg3shoc_127 test in the > rt_ccpp_dtc.conf create/verify test. I ran these separately after the fact, > and they passed. > > rt_ccpp_dtc_create_redo.log > https://github.com/NCAR/fv3atm/files/4404221/rt_ccpp_dtc_create_redo.log > rt_ccpp_dtc_verify_redo.log > https://github.com/NCAR/fv3atm/files/4404223/rt_ccpp_dtc_verify_redo.log > > @climbfuji https://github.com/climbfuji @mzhangw > https://github.com/mzhangw @ligiabernardet > https://github.com/ligiabernardet > This would be ready to merge into dtc/develop, but we should change the > target to mzhangw fork, right? Which branch? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#18 (comment)>, or > unsubscribe > https://github.com/notifications/unsubscribe-auth/AG7TW2S3D4WZCUJ6JHQYSBTRKDFJHANCNFSM4JZD2OFQ > . >

Yes. All other associated PRs are listed in a comment above.

@climbfuji
Copy link
Collaborator

@mzhangw @grantfirl @ligiabernardet at the last CCPP meeting on Thursday we said that I should create the umbrella PR in order to present all of this to EMC at some point. I am fine with you doing it, but I want to avoid duplication of work. You'll have to keep it up to date with whatever is merged into EMC's develop branches and NCAR's master branches in order to create the PR for EMC.

@climbfuji
Copy link
Collaborator

climbfuji commented Apr 7, 2020

This PR was merged into the dtc/hwrf-physics branch in PR #18. It didn't change the answer for the existing tests and can thus be merged into the EMC develop or NCAR dtc/develop branch when needed. Closing this PR.

@climbfuji climbfuji closed this Apr 16, 2020
SamuelTrahanNOAA pushed a commit to SamuelTrahanNOAA/fv3atm that referenced this pull request May 8, 2020
…bdhpbl

Correction: Diag%hpbl => Tbd%hpbl
SamuelTrahanNOAA pushed a commit to SamuelTrahanNOAA/fv3atm that referenced this pull request Jan 7, 2022
SamuelTrahanNOAA pushed a commit to SamuelTrahanNOAA/fv3atm that referenced this pull request Jun 13, 2022
* update FMS to point to tag 2019.01 of NOAA-GFDL
* update stochastic_physics to point to hash 1745422af76d830757cd6035b6ea101e92b4cac1 @pjpegion
* update of path to CCPP physics library for CCPP regression tests
* add -Wall to compiler flags for GNU compilers
* remove warnings for non-existent include directories for GNU compiler
* update of regression testing scripts to detect errors in tests/run_test.sh
* new regression tests fv3_ccpp_gfs_v15p2, fv3_ccpp_gfs_v15p2_debug, fv3_ccpp_gfs_v16beta, fv3_ccpp_gfs_v16beta_debug
* update of rt.conf: remove Cheyenne.intel entries (so that rt.conf is an EMC-maintained regression test configuration; move Cheyenne Intel tests into a separate file that resides in the NCAR dtc/develop branch)
* make compile_cmake.sh work with Cheyenne Intel/GNU, and bugfixes for compile_cmake.sh
* allow environment variable NEMS_MACHINE to overwrite (or set) MACHINE_ID (see NCAR#20 for the corresponding PR for the ufs_public_release branch)
* new regression test fv3_ccpp_gsd_sar_25km_debug (but not exercised in default rt.conf)
* updates for jet and gaea (note: only supporting rt.sh, not NEMSCompsetRun)
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