NCAR / ccpp-physics

GFS physics for CCPP
Other
59 stars 146 forks source link

Code update for radiation_clouds.f and GFS_rrtmg_pre.F90 #858

Closed Qingfu-Liu closed 2 years ago

Qingfu-Liu commented 2 years ago

Clear up the code of the cloud properties for radiation calculation. Changes in two program: radiation_clouds.f GFS_rrtmg_pre.F90

Qingfu-Liu commented 2 years ago

Regression tests not finished yet, documents also need a little more work

dustinswales commented 2 years ago

@Qingfu-Liu All of the changes look good to me. Thanks for doing this! A few small comments/suggestion: ) Can you output the sub-fields of "clouds" from radiation_clouds_prop? So instead of clouds(:,:,1:10), cld_frac, cld_lwp, etc, ... ) It might be good time to rename the progcld() routines to something more physical? Or maybe consistent with suffixes used by the "impphysics" flag? *) Remove instances of magic numbers in cloud overlap logic? Just as for the impphysics flag, there are interstitials for each of the iovr and idcor flags, defined in GFS_typedefs.

Qingfu-Liu commented 2 years ago

Hi Dustin, Great suggestions. I will work on those items. Qingfu

On Wed, Feb 9, 2022 at 5:16 PM dustinswales @.***> wrote:

@Qingfu-Liu https://github.com/Qingfu-Liu All of the changes look good to me. Thanks for doing this! A few small comments/suggestion: ) Can you output the sub-fields of "clouds" from radiation_clouds_prop? So instead of clouds(:,:,1:10), cld_frac, cld_lwp, etc, ... ) It might be good time to rename the progcld() routines to something more physical? Or maybe consistent with suffixes used by the "impphysics" flag? *) Remove instances of magic numbers in cloud overlap logic? Just as for the impphysics flag, there are interstitials for each of the iovr and idcor flags, defined in GFS_typedefs.

— Reply to this email directly, view it on GitHub https://github.com/NCAR/ccpp-physics/pull/858#issuecomment-1034246490, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGTS6UXPHPBL5X2HDUHSBQTU2LRTHANCNFSM5N6T6UHA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

Qingfu-Liu commented 2 years ago

Hello All, Need help to proceed forward: My question is how to upload the new files to the pull request. I have updated the code for the following files: radiation_clouds.f GFS_rrtmg_pre.F90 GFS_rrtmg_pre.meta GFS_cloud_diagnostics.F90 GFS_cloud_diagnostics.meta The files are on Hera: /scratch1/NCEPDEV/global/Qingfu.Liu/ufs-weather-model/FV3/ccpp/physics/physics One way I saw is to download these files to my local computer, then upload these files to the pull request. Not sure if doing this will change the FORTRAN code format. Is there other standard procedure to update the code to the pull request? Thank you very much. Qingfu

SamuelTrahanNOAA commented 2 years ago

If you push your files to the remote feature/codeupdate branch, then it will update the pull request (PR). The PR is literally just a request to NCAR to pull your changes from your branch in your fork back to the "main" branch in NCAR's repository. Hence, updating your branch will update the pull request.

Qingfu-Liu commented 2 years ago

Sam, Thank you very much. I will do it. Qingfu

Qingfu-Liu commented 2 years ago

Hi Sam, I try to push the individual file to the remote feature/codeupdate branch, and got the following error message:

@. physics]$ git remote add GFS_cloud_diagnostics.F90 feature/codeupdate fatal: remote GFS_cloud_diagnostics.F90 already exists. @. physics]$ git push GFS_cloud_diagnostics.F90 feature/codeupdate fatal: repository ' https://github.com/Qingfu-Liu/ccpp-physics/tree/feature/codeupdate/physics/' not found (by the way, I can access the http site via google browser) What should I do? Thank you very much. Qingfu

On Mon, Feb 14, 2022 at 7:55 AM Samuel Trahan (NOAA contractor) < @.***> wrote:

If you push your files to the remote feature/codeupdate branch, then it will update the pull request (PR). The PR is literally just a request to NCAR to pull your changes from your branch in your fork back to the "main" branch in NCAR's repository. Hence, updating your branch will update the pull request.

— Reply to this email directly, view it on GitHub https://github.com/NCAR/ccpp-physics/pull/858#issuecomment-1039048707, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGTS6UREKWRAWKZ3ZFHOG33U3D3CBANCNFSM5N6T6UHA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

SamuelTrahanNOAA commented 2 years ago

Wrong command. The "git remote add" will make another fork of the repository visible to your working copy. You want "git add" instead. I suggest you find a good git cheat sheet or tutorial. Gitlab has a good one:

https://docs.gitlab.com/ee/gitlab-basics/start-using-git.html#add-and-commit-local-changes

Also, if you're using a NOAA account, you may need to log in with ssh. This is the github page explaining how. (The process is different for gitlab.)

https://docs.github.com/en/authentication/connecting-to-github-with-ssh

Qingfu-Liu commented 2 years ago

Sam, Thank you very much. I am learning those commands. Please do not remove this draft pull, I am practicing the command to see if I can upload my new changes to the remote repository Qingfu

grantfirl commented 2 years ago

@Qingfu-Liu Are this pull request and #862 identical? If so, which do you want to keep?

Qingfu-Liu commented 2 years ago

Hi Grant, The two pulls are identical now. I have trouble uploading the individual file before due to the conflicts, so I create a new pull request. I guess we can remove the old one, and keep the new one Qingfu

On Tue, Feb 15, 2022 at 11:03 AM Grant Firl @.***> wrote:

@Qingfu-Liu https://github.com/Qingfu-Liu Are this pull request and #862 https://github.com/NCAR/ccpp-physics/pull/862 identical? If so, which do you want to keep?

— Reply to this email directly, view it on GitHub https://github.com/NCAR/ccpp-physics/pull/858#issuecomment-1040458897, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGTS6UQ4NVZJRVPDBR6ZCHTU3J2MFANCNFSM5N6T6UHA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

grantfirl commented 2 years ago

Closed in favor of #862