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

cam6_3_159: Diagnostic rainbows and GPU fixes in PUMAS microphysics #702

Merged
merged 12 commits into from
Apr 26, 2024

Conversation

Katetc
Copy link
Collaborator

@Katetc Katetc commented Nov 19, 2022

Fixes #683 Add Diagnostic Rainbow Calculation
Partially addresses #1007 Broken PUMAS GPU code and GPU regression test by bringing in an updated PUMAS external tag with fixed GPU code. However, the needed ccs_config tag ( ccs_config_cesm0.0.99 ) caused a failure in the single column tests right now. So the updates to the ccs_config external will be left for the next GPU fixing tag.

RAINBOWS!!! I'M NOT KIDDING!!!!111!
Also a new PUMAS tag that fixes GPU OpenACC calls. B4b

Does not change answers.

@Katetc Katetc self-assigned this Nov 19, 2022
@cacraigucar cacraigucar added the enhancement New feature or request label Nov 21, 2022
Copy link
Collaborator

@andrewgettelman andrewgettelman left a comment

Choose a reason for hiding this comment

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

Thanks Kate. This looks good. I ran with CAM and CAM_DEV and verify that I can reproduce the results I was expecting. Good from my side.

@Katetc Katetc changed the title Katetc/rainbows pr Adding in diagnostic rainbows to microphysics Nov 28, 2022
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Looks good! Just have a few minor change requests (it looks like a lot simply because I repeated the same comments for both cam and cam_dev).

src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam_dev/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam_dev/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam_dev/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam_dev/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
@nusbaume nusbaume removed the request for review from peverwhee September 1, 2023 13:00
@Katetc Katetc requested a review from nusbaume April 1, 2024 22:36
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Everything looks great to me now, thanks!

Copy link
Collaborator

@cacraigucar cacraigucar left a comment

Choose a reason for hiding this comment

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

My comments in cam/micro_pumas_cam.F90 also apply to cam_dev/micro_pumas_cam.F90

src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
src/physics/cam/micro_pumas_cam.F90 Outdated Show resolved Hide resolved
@Katetc Katetc changed the title Adding in diagnostic rainbows to microphysics Diagnostic rainbows and GPU fixes in PUMAS microphysics Apr 17, 2024
@Katetc Katetc requested a review from cacraigucar April 22, 2024 18:11
@cacraigucar cacraigucar changed the title Diagnostic rainbows and GPU fixes in PUMAS microphysics cam6_3_159: Diagnostic rainbows and GPU fixes in PUMAS microphysics Apr 23, 2024
do i = 1, ncol

! Rainbows currently calculated on the grid, not subcolumn specific
do i = 1, ngrdcol
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I do not use SCM or SILHS (e.g., just the default F2000 compset), does ngrdcol equal to ncol in this case?

Copy link
Collaborator Author

@Katetc Katetc Apr 26, 2024

Choose a reason for hiding this comment

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

If subcolumns are not turned on (SILHS or any other subcolumn source) then ngrdcol is equal to ncol. The SCM issue was a result of the ccs_config tag, but I'm not sure why.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the clarification. That is good to know.

Externals.cfg Outdated
@@ -1,5 +1,5 @@
[ccs_config]
tag = ccs_config_cesm0.0.99
tag = ccs_config_cesm0.0.85
Copy link
Collaborator

Choose a reason for hiding this comment

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

By changing the tag from ccs_config_cesm0.0.99 to ccs_config_cesm0.0.85, the PUMAS GPU code won't be compiled correctly. Will the ccs_config_cesm0.0.99 tag be brought separately later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The ccs_config_cesm0.0.99 tag was causing the single column model test to fail. I'm not sure why, and I don't think it's in the scope of this tag to figure it out. When you work with Brian to bring in the GPU test suite, hopefully you can find and address the problem. Or, if somebody else updates the externals for CAM, they will need to fix this test. I will update the description to note this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, thanks for the clarification. Do you have the steps to reproduce the failed single column model test?

@cacraigucar
Copy link
Collaborator

@Katetc - Which test is the failing scm test? I would suggest you add a prealpha test for it as well, so that when @fischer-ncar updates the ccs_config (which I think he has already done, or may be doing with the next alpha tag), that the test fails at that level. I assume that when you say scm, this is a SCAM test. We want to make sure that we don't break SCAM with this next beta tag.

@Katetc
Copy link
Collaborator Author

Katetc commented Apr 26, 2024

@Katetc - Which test is the failing scm test? I would suggest you add a prealpha test for it as well, so that when @fischer-ncar updates the ccs_config (which I think he has already done, or may be doing with the next alpha tag), that the test fails at that level. I assume that when you say scm, this is a SCAM test. We want to make sure that we don't break SCAM with this next beta tag.

HI Cheryl, this test fails with the newest ccs_config tag: SCT_D_Ln7.T42_T42_mg17.QPC5.derecho_intel.cam-scm_prep and passes with the current one. So I'm rolling back the ccs_config tag in this PR. I'll open an issue about this. And I can add a prealpha marking to this test as well.

@Katetc
Copy link
Collaborator Author

Katetc commented Apr 26, 2024

I have added this issue to be addressed regarding the ccs_config tag: #1017

@Katetc
Copy link
Collaborator Author

Katetc commented Apr 26, 2024

I have added this issue to be addressed regarding unexpected test DIFFs for HEMCO: #1018

@Katetc Katetc merged commit 46ff80c into ESCOMP:cam_development Apr 26, 2024
gold2718 pushed a commit to gold2718/CAM that referenced this pull request Oct 16, 2024
Merge pull request ESCOMP#702 from PUMASDevelopment/katetc/rainbows_pr

cam6_3_159: Diagnostic rainbows and GPU fixes in PUMAS microphysics
Fixes ESCOMP#683 Add Diagnostic Rainbow Calculation
Partially addresses ESCOMP#1007 Broken PUMAS GPU code and GPU regression test by bringing in an updated PUMAS external tag with fixed GPU code. However, the needed ccs_config tag ( ccs_config_cesm0.0.99 ) caused a failure in the single column tests right now. So the updates to the ccs_config external will be left for the next GPU fixing tag.

RAINBOWS!!! I'M NOT KIDDING!!!!111!
Also a new PUMAS tag that fixes GPU OpenACC calls. B4b

Does not change answers.

ESCOMP commit: 46ff80c
gold2718 pushed a commit to gold2718/CAM that referenced this pull request Oct 16, 2024
Merge pull request ESCOMP#702 from PUMASDevelopment/katetc/rainbows_pr

cam6_3_159: Diagnostic rainbows and GPU fixes in PUMAS microphysics
Fixes ESCOMP#683 Add Diagnostic Rainbow Calculation
Partially addresses ESCOMP#1007 Broken PUMAS GPU code and GPU regression test by bringing in an updated PUMAS external tag with fixed GPU code. However, the needed ccs_config tag ( ccs_config_cesm0.0.99 ) caused a failure in the single column tests right now. So the updates to the ccs_config external will be left for the next GPU fixing tag.

RAINBOWS!!! I'M NOT KIDDING!!!!111!
Also a new PUMAS tag that fixes GPU OpenACC calls. B4b

Does not change answers.

ESCOMP commit: 46ff80c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Tag
Development

Successfully merging this pull request may close these issues.

5 participants