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

[production/RRFS.v1] fix for inline post when using RUC LSM #877

Conversation

JiliDong-NOAA
Copy link
Contributor

Description

@ericaligo-NOAA found an issue where inline post does not correctly output soil temperature and moisture in grib2 when using RUC LSM. We tracked this to post_fv3 where iSF_SURFACE_PHYSICS is not updated for RUC LSM. This parameter is used in UPP SURFACE.f for soil variables output. The initial iSF_SURFACE_PHYSICS = 2 in post_fv3 and should be updated to 3 when RUC LSM is used. In this PR we set iSF_SURFACE_PHYSICS =3 when nsoil=9 and successfully output the 9 level soil variables.

A PR to ufs-weather-model will be submitted later.

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)

  • fixes #<issue_number>
  • fixes noaa-emc/fv3atm/issues/<issue_number>

Testing

How were these changes tested?
What compilers / HPCs was it tested with?
Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
Have the ufs-weather-model regression test been run? On what platform?

  • Will the code updates change regression test baseline? If yes, why? Please show the baseline directory below.
  • Please commit the regression test log files in your ufs-weather-model branch

Dependencies

If testing this branch requires non-default branches in other repositories, list them.
Those branches should have matching names (ideally)

Do PRs in upstream repositories need to be merged first?
If so add the "waiting for other repos" label and list the upstream PRs

  • waiting on noaa-emc/nems/pull/<pr_number>
  • waiting on noaa-emc/fv3atm/pull/<pr_number>

Requirements before merging

  • All new code in this PR is tested by at least one unit test
  • All new code in this PR includes Doxygen documentation
  • All new code in this PR does not add new compilation warnings (check CI output)

@JiliDong-NOAA
Copy link
Contributor Author

could you add @junwang-noaa and @WenMeng-NOAA to review this PR? Thanks

@WenMeng-NOAA
Copy link
Contributor

@JiliDong-NOAA Thanks for catching this bug. Your fix looks good to me.

@JiliDong-NOAA
Copy link
Contributor Author

following @WenMeng-NOAA 's suggestion, the PR has been updated by passing "landsfcmdl" to "iSF_SURFACE_PHYSICS" in post_fv3.

io/post_fv3.F90 Outdated
@@ -592,6 +593,7 @@ subroutine set_postvars_fv3(wrt_int_state,grid_id,mype,mpicomp)
!
integer i, ip1, j, l, k, n, iret, ibdl, rc, kstart, kend
integer i1,i2,j1,j2,k1,k2
integer landsfcmdl
Copy link
Contributor

Choose a reason for hiding this comment

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

@JiliDong-NOAA Do you think this allocation might be unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure about that. Just to be safe I guess.

@DusanJovic-NOAA
Copy link
Collaborator

@JiliDong-NOAA Looks like code changes from this PR have been added to #869. Please confirm this is correct and if it is close this PR.

@JiliDong-NOAA
Copy link
Contributor Author

@JiliDong-NOAA Looks like code changes from this PR have been added to #869. Please confirm this is correct and if it is close this PR.

That's correct and I'll have this one closed.

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.

3 participants