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.

Sign up for GitHub

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

Jump to bottom

Accumulation of changes to RRTMGP #515

Merged
merged 29 commits into from
Apr 30, 2021
Merged

Accumulation of changes to RRTMGP #515

merged 29 commits into from
Apr 30, 2021

Conversation

dustinswales
Copy link
Collaborator

@dustinswales dustinswales commented Apr 9, 2021 •
edited by climbfuji
Loading

Description

This PR contains improvements and bug fixes to the RRTMGP enabled ccpp-physics.

  • Bug fix (indexing error) for temperature being passed to GP. (Only exercised in rare cases in EMC's C768L127 configuration).
  • New coupling of RRTMGP to the RRFS physics. Based on fv3_regional_quilt using FV3_GFS_v15_thompson_mynn_RRTMGP.
  • Revised approach to the LW flux adjustment using the full vertical profile of the Jacobian of the upwelling LW. Previously only the surface flux was adjusted.
  • Move allocation of GP interstitial DDTs from the LW scheme to GFS_typedefs.F90 interstitial_create/reset.

Issue(s) addressed

N/A

Testing

This was tested on Hera using the Intel compilers. There are no changes to existing baselines. A new baseline needs to be added for the new RT, fv3_regional_quilt_RRTMGP.

Edit from @climbfuji:

  • ALL RRTMGP tests change results
  • Because of the bugfix for jet in ccpp-physics (previously, the compiler flag adjustments in FV3/ccpp/physics/CMakeLists.txt were ignored), most if not all results on jet change
  • A number of bugfixes were required, some of which only showed up with the GNU compiler

Regression testing on tier-1 platforms:

  • hera.intel
  • hera.gnu
  • orion.intel - skip because of ongoing filesystem (/work) problems
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel - log was lost due to bug in auto-rt, attaching the rt_auto log below instead.
  • jet.intel
  • wcoss_cray
  • wcoss_dell_p3

gaea_intel_rt_auto_20210429173006.log

Dependencies

NOAA-EMC/fv3atm#275
NCAR/ccpp-physics#614

@RobertPincus
Copy link

Maybe just to emphasize that the title of the PR describes one change but the PR includes at least four changes.

@dustinswales dustinswales changed the title Add l wadj full profile Accumulation of changes to RRTMGP Apr 9, 2021
climbfuji
climbfuji requested changes Apr 28, 2021
View reviewed changes
tests/fv3_conf/gfs_v15_run.IN Outdated Show resolved Hide resolved
tests/fv3_conf/gfs_v16_run.IN Outdated Show resolved Hide resolved
tests/fv3_conf/gsd_run.IN Show resolved Hide resolved
tests/fv3_conf/regional_run.IN Outdated Show resolved Hide resolved
tests/parm/v16_c96_rrtmgp.nml.IN Outdated Show resolved Hide resolved
tests/tests/fv3_gfs_v16_RRTMGP Show resolved Hide resolved
@climbfuji climbfuji added Baseline Updates Current baselines will be updated. New Input Data Req'd This PR requires new data to be sync across platforms labels Apr 28, 2021
@climbfuji climbfuji added the Waiting for Reviews The PR is waiting for reviews from associated component PR's. label Apr 28, 2021
@climbfuji
Copy link
Collaborator

Update on these PRs:

  • several errors in the coderegression tests were addressed during the code review and while generating baselines yesterday
  • two RRTMGP regression tests were commented out due to failures, among them the RRTMGP threading test
  • the code does not compile on jet, and while we have created baselines on all other platforms we cannot proceed without knowing how the fix for jet impacts the code and the (RRTMGP) regression test results
  • Note: because changes were made to detecting OpenMP for CCPP, @climbfuji compared the runtime of the existing control threading test (fv3_2threads) with the current trunk, and the runtime is identical (i.e. good)

The UFS code managers discussed, if the PR is not finalized and merged this afternoon MT, it will be postponed and we move on to the next PR.

@junwang-noaa @DusanJovic-NOAA @dustinswales @RobertPincus

@dustinswales
Copy link
Collaborator Author

@climbfuji
The GP RT w/ threading worked for me. I had to make a change to rt.conf for the test to work. Working on the c192l127test now

@climbfuji
Copy link
Collaborator

climbfuji commented Apr 29, 2021 •
edited
Loading

@climbfuji
The GP RT w/ threading worked for me. I had to make a change to rt.conf for the test to work. Working on the c192l127test now

You need to revert this change. Adding fv3 means that you are overwriting the single-thread results with the two-thread results. The 2thread test must pass against the single thread results.

More info: the fv3 column is used to determine whether a test is run when baselines are created. Tests without fv3 are only run when results are verified against existing baselines.

@dustinswales
Copy link
Collaborator Author

@

@climbfuji
The GP RT w/ threading worked for me. I had to make a change to rt.conf for the test to work. Working on the c192l127test now

You need to revert this change. Adding fv3 means that you are overwriting the single-thread results with the two-thread results. The 2thread test must pass against the single thread results.

More info: the fv3 column is used to determine whether a test is run when baselines are created. Tests without fv3 are only run when results are verified against existing baselines.

Gotcha. Will revert.

@climbfuji
Copy link
Collaborator

Note: CI tests passed for commit a9a3887.

@climbfuji
Copy link
Collaborator

Regression tests passed on all tier-1 platforms, except Orion: filesystem issues /work/ not responding, cannot run tests. I will start the merge with ccpp-physics.

@github-actions
Copy link

@dustinswales please bring these up to date with respective authoritative repositories

  • fv3 NOT up to date

climbfuji
climbfuji approved these changes Apr 30, 2021
View reviewed changes
Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

I fixed the submodule pointer for fv3atm, ready to merge.

@junwang-noaa junwang-noaa merged commit 2fc0ce3 into ufs-community:develop Apr 30, 2021
@dustinswales
Copy link
Collaborator Author

I fixed the submodule pointer for fv3atm, ready to merge.

Thanks!

@dustinswales dustinswales deleted the addLWadj_fullProfile branch May 10, 2021 17:20
epic-cicd-jenkins pushed a commit that referenced this pull request Apr 17, 2023
@JeffBeck-NOAA
Add /atmos to HPSS path for GFS grib2 files after certain date (#515)
4b86b14 
* Add atmos to HPSS path for GFS grib2 files after certain date

* Add slash_atmos_or_null
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RobertPincus RobertPincus RobertPincus left review comments

@junwang-noaa junwang-noaa junwang-noaa approved these changes

@Qingfu-Liu Qingfu-Liu Qingfu-Liu approved these changes

@climbfuji climbfuji climbfuji approved these changes

@DusanJovic-NOAA DusanJovic-NOAA Awaiting requested review from DusanJovic-NOAA

Assignees
No one assigned
Labels
Baseline Updates Current baselines will be updated. New Input Data Req'd This PR requires new data to be sync across platforms Waiting for Reviews The PR is waiting for reviews from associated component PR's.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dustinswales @RobertPincus @BrianCurtis-NOAA @climbfuji @Qingfu-Liu @junwang-noaa