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

pressure is not density and weasdi is not snodi + #739 and #742 #736

Merged
merged 22 commits into from
Dec 21, 2023
Merged

pressure is not density and weasdi is not snodi + #739 and #742 #736

merged 22 commits into from
Dec 21, 2023

Conversation

SamuelTrahanNOAA
Copy link
Contributor

@SamuelTrahanNOAA SamuelTrahanNOAA commented Dec 15, 2023 •
edited
Loading

Description

PRs #739 and #742 are combined into this one

Fixes two bugs:

  1. CLM Lake was using near-surface pressure in place of near-surface density.
  2. GFS_diagnostics was outputting weasdi (water equivalent snow depth over ice) in place of snodi (snow depth over ice)

Fixes are from @tanyasmirnova

Issue(s) addressed

These PRs are included in this one. They can be closed when this PR is merged:

Due to #742:

Many ccpp-physics issues are closed, and two pull requests. For a full list, see ufs-community/ccpp-physics#139

Testing

How were these changes tested?

A failing RRFS test case.

What compilers / HPCs was it tested with?

Intel Hera

Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)

Existing tests already exercise this code well. Sadly, no test can catch every possible failure.

Have the ufs-weather-model regression test been run? On what platform?

Hera.

Will the code updates change regression test baseline? If yes, why? Please show the baseline directory below.

All tests with "hrrr" or "clm_lake" in their name.

/scratch1/BMC/zrtrr/Samuel.Trahan/lakes/RT/NEMSfv3gfs/develop-20231207

Please commit the regression test log files in your ufs-weather-model branch

Done.

Dependencies

@SamuelTrahanNOAA SamuelTrahanNOAA mentioned this pull request Dec 15, 2023
Merged
35 tasks
@DusanJovic-NOAA
Copy link
Collaborator

Since you are touching GFS_diagnostics.F90 can you please fix the typo in the description of wilting point 'wilt':

fv3atm/ccpp/driver/GFS_diagnostics.F90

Line 2181 in a82381c

ExtDiag(idx)%desc = 'wiltimg point (volumetric)'

@SamuelTrahanNOAA
Copy link
Contributor Author

Since you are touching GFS_diagnostics.F90 can you please fix the typo in the description of wilting point 'wilt':

fv3atm/ccpp/driver/GFS_diagnostics.F90

Line 2181 in a82381c

ExtDiag(idx)%desc = 'wiltimg point (volumetric)'

Will that change the results of any test?

Changing snodi affects nothing since no test outputs snodi.

@DusanJovic-NOAA
Copy link
Collaborator

Since you are touching GFS_diagnostics.F90 can you please fix the typo in the description of wilting point 'wilt':

fv3atm/ccpp/driver/GFS_diagnostics.F90

Line 2181 in a82381c

ExtDiag(idx)%desc = 'wiltimg point (volumetric)'

Will that change the results of any test?

Changing snodi affects nothing since no test outputs snodi.

I noticed the typo by looking at ncdump, so yes it will change the output, but I have no idea how many tests are affected. Ignore it, since this PR does not change outputs.

@SamuelTrahanNOAA
Copy link
Contributor Author

@JiliDong-NOAA - Please confirm that I merged your changes into this branch correctly.

@SamuelTrahanNOAA SamuelTrahanNOAA changed the title pressure is not density and weasdi is not snodi pressure is not density and weasdi is not snodi + #739 and #742 Dec 19, 2023
@JiliDong-NOAA
Copy link
Contributor

@JiliDong-NOAA - Please confirm that I merged your changes into this branch correctly.

looks good too.

@jkbk2004
Copy link
Collaborator

@SamuelTrahanNOAA physics pr was merged.

@dustinswales dustinswales mentioned this pull request Dec 21, 2023
Merged
@SamuelTrahanNOAA
Copy link
Contributor Author

This PR is ready for final review and merge. I reverted .gitmodules and pointed ccpp/physics to ufs-community's ufs/dev.

@jkbk2004 jkbk2004 merged commit 87a0cca into NOAA-EMC:develop Dec 21, 2023
zach1221 pushed a commit to ufs-community/ufs-weather-model that referenced this pull request Dec 21, 2023
@SamuelTrahanNOAA
CLM Lake pressure is not density and GFS Diagnostics: weasdi is not s…
991d652 
…nodi + #2047, #2053, and #2056 (#2044)

FV3 diagnostic fixes, CCPP fixes for model crashes, new PR template

- UFS:
    - commit message in PR template (#2053)
    - fix hercules crashes (#2015)
- CMEPS & FV3: Bad data from in CCPP CLM Lake physics scheme caused model crashes
    - Communicate changes to lake ice (Closes #2055, NOAA-EMC/CMEPS#105, NOAA-EMC/fv3atm#741) 
    - unit mismatch (NOAA-EMC/fv3atm#736)
- FV3: correct errors in diagnostic calculations
    - snodi had weasdi data in it (NOAA-EMC/fv3atm#736)
    - revisions to RUC LSM snowfall melting and accumulation (NOAA-EMC/fv3atm#739)
@grantfirl grantfirl mentioned this pull request Mar 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DusanJovic-NOAA DusanJovic-NOAA DusanJovic-NOAA approved these changes

@tanyasmirnova tanyasmirnova tanyasmirnova approved these changes

@jkbk2004 jkbk2004 jkbk2004 approved these changes

@BrianCurtis-NOAA BrianCurtis-NOAA Awaiting requested review from BrianCurtis-NOAA

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@SamuelTrahanNOAA @DusanJovic-NOAA @JiliDong-NOAA @jkbk2004 @tanyasmirnova