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

Develop drag suite intent mods -- Changed UGWP diagnostic variable declaration intents from 'out' to 'inout' #1612

Closed
wants to merge 6 commits into from
Closed

Develop drag suite intent mods -- Changed UGWP diagnostic variable declaration intents from 'out' to 'inout' #1612

wants to merge 6 commits into from

Conversation

mdtoyNOAA
Copy link
Contributor

@mdtoyNOAA mdtoyNOAA commented Feb 14, 2023 •
edited by DeniseWorthen
Loading

Description

This pull request fixes Issue #37 of the ufs-community/ccpp-physics repository. The UGWP diagnostic variables (i.e., dusfc_ms, dvsfc_ms, tdaux2d_ms, datauy2d_ms, etc.) are declared with "intent(out)" and initialized in module unified_ugwp.F90, then they are passed to subroutine "drag_suite_run" (in drag_suite.F90). In this subroutine, the UGWP diagnostic variables should have been declared with "intent(inout)" instead of "intent(out)". This PR fixes this.
This PR does not change any results.

Top of commit queue on: TBD

Input data additions/changes

  • No changes are expected to input data.
  • There will be new input data.
  • Input data will be updated.

Anticipated changes to regression tests:

  • No changes are expected to any regression test.
  • Changes are expected to the following tests:

Subcomponents involved:

  • AQM
  • CDEPS
  • CICE
  • CMEPS
  • CMakeModules
  • FV3
  • GOCART
  • HYCOM
  • MOM6
  • NOAHMP
  • WW3
  • stochastic_physics
  • none

Combined with PR's (If Applicable):

Commit Queue Checklist:

  • Link PR's from all sub-components involved
  • Confirm reviews completed in sub-component PR's
  • Add all appropriate labels to this PR.
  • Run full RT suite on either Hera/Cheyenne with both Intel/GNU compilers
  • Add list of any failed regression tests to "Anticipated changes to regression tests" section.

Linked PR's and Issues:

Dependent pull requests:

Related issue:

Testing Day Checklist:

  • This PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR.
  • Move new/updated input data on RDHPCS Hera and propagate input data changes to all supported systems.

Testing Log (for CM's):

  • RDHPCS
    • Intel
      • Hera
      • Orion
      • Jet
      • Gaea
      • Cheyenne
    • GNU
      • Hera
      • Cheyenne
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
    • Completed
  • opnReqTest
    • N/A
    • Log attached to comment

@jkbk2004 jkbk2004 mentioned this pull request Feb 15, 2023
Merged
37 tasks
@jkbk2004
Copy link
Collaborator

@mdtoyNOAA We are currently working on #1529. As discussed before, this pr can be combined to #1597. I will check again after we merge in #1529. We can coordinate with @binli2337 then.

@mdtoyNOAA
Copy link
Contributor Author

@mdtoyNOAA We are currently working on #1529. As discussed before, this pr can be combined to #1597. I will check again after we merge in #1529. We can coordinate with @binli2337 then.

@jkbk2004 Sounds good.

@DeniseWorthen
Copy link
Collaborator

It does not appear that the GNU suite was run. Also FV3 is out of date.

@mdtoyNOAA
Copy link
Contributor Author

@jkbk2004 I've merged the latest changes to "ufs-weather-model" and "FV3ATM" in my fork. Would you like me to retest Intel and test GNU?

@jkbk2004
Copy link
Collaborator

@mdtoyNOAA you are bringing in the change from gsl fork branches. It's a bit tricky. Easiest solution is to go with community develop and emc fv3 develop. Since this pr is small change, I think it's doable to re-create pr. What do you think? we are still working on #1529. There is enough time.

@mdtoyNOAA
Copy link
Contributor Author

@jkbk2004 Actually, I am not bringing in changes from the GSL fork branches. Although I did originally fork my ufs-weather-model repo from NOAA-GSL, the branches related to this PR are strictly based on the ufs-community repos. To double check this, I just cloned my ufs-weather-model repo into a fresh directory and "git diffed" each component, i.e., ufs-weather-model, fv3atm, and ccpp/physics, and they are all synched (except for my "inout" mods) to the ufs-community repos/branches. So, I shouldn't need to redo the PR. Please let me know if you agree.

@jkbk2004
Copy link
Collaborator

@jkbk2004 Actually, I am not bringing in changes from the GSL fork branches. Although I did originally fork my ufs-weather-model repo from NOAA-GSL, the branches related to this PR are strictly based on the ufs-community repos. To double check this, I just cloned my ufs-weather-model repo into a fresh directory and "git diffed" each component, i.e., ufs-weather-model, fv3atm, and ccpp/physics, and they are all synched (except for my "inout" mods) to the ufs-community repos/branches. So, I shouldn't need to redo the PR. Please let me know if you agree.

cool! sounds at least synching branch is working. Once we don't see conflictions to resolve, we are fine.

@jkbk2004
Copy link
Collaborator

@mdtoyNOAA hera is very slow from yesterday. Do you have access to Cheyenne? I can help to make a quick test on cheyenne with gnu.

@mdtoyNOAA
Copy link
Contributor Author

@mdtoyNOAA hera is very slow from yesterday. Do you have access to Cheyenne? I can help to make a quick test on cheyenne with gnu.

@jkbk2004 No, I don't have access to Cheyenne.

@jkbk2004
Copy link
Collaborator

@mdtoyNOAA let me give a quick test on cheyenne. Anyway, we like to start working on this pr today with combining to #1597

@DeniseWorthen DeniseWorthen added the No Baseline Change No Baseline Change label Feb 17, 2023
@jkbk2004
Copy link
Collaborator

RegressionTests_cheyenne.gnu.log

cheyenn.gnu reproduces

@jkbk2004
Copy link
Collaborator

@mdtoyNOAA Ok! #1529 was merged in. So, we can start working to combine this pr to #1597. Can you sync up ccpp and fv3 one more time? I will ask @binli2337 to point to your fv3 branch in his pr.

@mdtoyNOAA
Copy link
Contributor Author

@jkbk2004 I am working on it right now. Can you confirm that there have been changes to the submodules 'WW3' and 'upp'?

@jkbk2004
Copy link
Collaborator

Yes, upp was update: https://github.com/NOAA-EMC/UPP/tree/b37f8ab7b0f298346d79a37e0c5d4a64037fd4d4
ww3 no update and the hash is still in last month: https://github.com/NOAA-EMC/WW3/tree/b93b9f00118f6fb316fd2289dff10b1455357f3f

@mdtoyNOAA
Copy link
Contributor Author

mdtoyNOAA commented Feb 17, 2023 via email

@jkbk2004
Copy link
Collaborator

Oh! you are correct. noah-mp was update.

@mdtoyNOAA
Copy link
Contributor Author

@jkbk2004 I updated my UPP branch and it looks like the latest hash is '84cp0b8f', not 'b37f8ab7'. Should I revert back to this older hash?

Gitlog below:

commit 84cb0b8ff4d72506edddff6204d00f39bc3f035a (HEAD, origin/develop, origin/HEAD)
Author: YaliMao-NOAA 53870326+YaliMao-NOAA@users.noreply.github.com
Date: Wed Feb 15 21:27:43 2023 -0500

update gtg code revision (#632)

Co-authored-by: yali mao <yali.mao@dlogin02.dogwood.wcoss2.ncep.noaa.gov>

commit ad56e5ca699728df7b300e3624807a6d21dcf186
Author: Gillian Petro 96886803+gspetro-NOAA@users.noreply.github.com
Date: Tue Feb 14 10:21:31 2023 -0500

Update documentation for variables in CALGUST.f and CALHEL.f (#630)

* Correct DEPTH param in CALHEL

* add def for LPBL and ZPBL

* reorder variables in header

---------

Co-authored-by: gspetro <gillian.s.petro@gmail.com>

commit 8c7ca6f36a0347b457e5de89b3b40c79a87ca353
Author: EricJames-NOAA eric.james@noaa.gov
Date: Mon Feb 13 18:09:04 2023 -0700

Small changes to exp2 ceiling and SLP output (#629)

* Correcting name of MAPS SLP variable in post_avblflds.xml, and removing from RRFS postxconfig file, and a slight change to exp2 ceiling diagnostic.

* Small change to MDLFLD to make sure REFD at -10C still gets output after getting rid of MSLMA output.

* Adding change logs.

* Adding NCEP local use for MSLMA

commit b37f8ab7b0f298346d79a37e0c5d4a64037fd4d4
Author: WenMeng-NOAA 48260754+WenMeng-NOAA@users.noreply.github.com
Date: Mon Feb 13 09:43:43 2023 -0500

@jkbk2004
Copy link
Collaborator

upp b37f8ab7 is the one we are using.

@mdtoyNOAA
Copy link
Contributor Author

@jkbk2004 Okay, I think I have it synched and pushed properly. Let me know if it looks okay. Thanks.

@jkbk2004
Copy link
Collaborator

@mdtoyNOAA one thing to confirm. this pr has not much to do with noahmp, right? Only code change comes from ccpp. So, @binli2337 can simply poin to your fv3 branch to combine the PRs, right?

@mdtoyNOAA
Copy link
Contributor Author

mdtoyNOAA commented Feb 18, 2023 via email

This was referenced Mar 2, 2023
Closed
Merged
@mdtoyNOAA mdtoyNOAA deleted the develop_drag_suite_intent_mods branch March 9, 2023 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
No Baseline Change No Baseline Change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possible correction to declaration intent for diagnostic variables in drag_suite.F90
3 participants
@mdtoyNOAA @jkbk2004 @DeniseWorthen