-
Notifications
You must be signed in to change notification settings - Fork 249
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
Bug fix for MERRA2 coupled Thompson microphysics and UPP update #1915
Conversation
regression test went well at /scratch1/NCEPDEV/global/Anning.Cheng/ufs-weather-model/tests |
@AnningCheng-NOAA It looks like your changes are actually in ccpp/physics
But there are no committed changes in your FV3 branch. Your PR to UFS needs point to your FV3 and ccpp branches. |
HI, Denise:
Here are the related PRs and Issues:
#1914
#1915
NOAA-EMC/fv3atm#700
ufs-community/ccpp-physics#109
…On Fri, Sep 22, 2023 at 11:00 AM Denise Worthen ***@***.***> wrote:
@AnningCheng-NOAA <https://github.com/AnningCheng-NOAA> It looks like
your changes are actually in ccpp/physics
diff --git a/physics/module_mp_thompson.F90 b/physics/module_mp_thompson.F90
index ca913c6e..271db11d 100644
--- a/physics/module_mp_thompson.F90
+++ b/physics/module_mp_thompson.F90
@@ -1509,6 +1509,14 @@ MODULE module_mp_thompson
enddo
endif
+ if (merra2_aerosol_aware) then
+ do k = kts, kte
+ nc(i,k,j) = nc1d(k)
+ nwfa(i,k,j) = nwfa1d(k)
+ nifa(i,k,j) = nifa1d(k)
+ enddo
+ endif
+
do k = kts, kte
qv(i,k,j) = qv1d(k)
qc(i,k,j) = qc1d(k)
But there are no committed changes in your FV3 branch. You'll need to make
a PR to ccpp and then to FV3 and then to UFS for your changes. Your PR to
UFS needs point to those two branches.
—
Reply to this email directly, view it on GitHub
<#1915 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIK6WS6QDUNUYUGWI3DX3WR2BANCNFSM6AAAAAA5CBYAX4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
HI, Denise:
Could you give me some instructions(command series) on how to make those
hash pointings right?
Thank you!
Anning
…On Fri, Sep 22, 2023 at 2:11 PM Denise Worthen ***@***.***> wrote:
Hi Anning. Thanks for adding the PR information. But the FV3 hash in this
PR (e4ead8a
<AnningCheng-NOAA/fv3atm@e4ead8a>)
does not appear to contain your ccpp branch. It points to ccpp hash
bbc5bf8
<AnningCheng-NOAA/fv3atm@bbc5bf8>.
I think it needs to point to your feature branch in ccpp 5377c7c
<AnningCheng-NOAA/ccpp-physics@5377c7c>
—
Reply to this email directly, view it on GitHub
<#1915 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIICPU63QZWJO4GYWNLX3XIDBANCNFSM6AAAAAA5CBYAX4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Start by getting your FV3 branch correct. I think you've missed one step. git clone https://github.com/AnningCheng-NOAA/fv3atm.git cd ccpp/phyics (git remote -v correctly shows you are in your fork) now, git status will show you've made an update to ccpp/physics
The next step is the step I think you missed. You need to add and commit that new hash: Once you do that, you should see your FV3 branch mrfd contains ccpp/physics with the correct hash. You can check by doing git submodule status --recursive which will show you
|
Once you've got your FV3 branch correct, you do the same process with your UFS branch. You need to both checkout your FV3 branch (which should now contain your ccpp/phyics) and add your FV3 to your ufs feature branch. |
Hi, @AnningCheng-NOAA . I think we may begin testing against your PR soon. Could you please sync up your ufs-wm and fv3atm branches to resolve conflicts? |
Zach:
done with the synchronization.
…On Thu, Oct 12, 2023 at 12:47 PM zach1221 ***@***.***> wrote:
Hi, @AnningCheng-NOAA <https://github.com/AnningCheng-NOAA> . I think we
may begin testing against your PR soon. Could you please sync up your
ufs-wm and fv3atm branches to resolve conflicts?
—
Reply to this email directly, view it on GitHub
<#1915 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIOSAZQ2M6LLPXUUJVDX7ANJ5ANCNFSM6AAAAAA5CBYAX4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@AnningCheng-NOAA looks like CICE, CMEPS, and stochastic physics not synced yet. |
Jong,
I have not modified those branches at all, not how mrfd is produced. Could
you give me some instructions or command series to sync them?
Anning
…On Thu, Oct 12, 2023 at 2:24 PM JONG KIM ***@***.***> wrote:
@AnningCheng-NOAA <https://github.com/AnningCheng-NOAA> looks like CICE,
CMEPS, and stochastic physics not synced yet.
—
Reply to this email directly, view it on GitHub
<#1915 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIKQEXD4EKUB3OEIGB3X7AYVBANCNFSM6AAAAAA5CBYAX4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I have just run |
@AnningCheng-NOAA you can take a look at "synching latest HEAD section" in https://github.com/ufs-community/ufs-weather-model/wiki/Making-code-changes-in-the-UFS-weather-model-and-its-subcomponents. But sounds like you just need to update hashes for CICE/CMEPS/Stochastic-physics submodules. Let me see if I can push to update hashes directly. |
@AnningCheng-NOAA Can you make to sync up ccpp physics with latest HEAD of https://github.com/ufs-community/ccpp-physics/tree/ufs/dev? That's where your code changes are originated. So you need to make sure code changes are reflected correctly in ufs-community/ccpp-physics#109. |
@AnningCheng-NOAA @grantfirl branches all looks synced up ok now. @WenMeng-NOAA baseline is expected to change with upp updates. We will go ahead to update baseline. |
@BrianCurtis-NOAA In your original testing of this PR on Acorn when you got the failures, the failures occurred at different lines in two different tries (not in debug mode). Is that right? That hints at somehow triggering an uninitialized variable or something like that. But, I don't have an explanation for a) why on Acorn and b) why this PR. |
@zach1221 Might also be worth testing GNU + debug on Hercules. |
I had the same error trying in debug mode with the PR branch. The test seems to fail at different places (in non-debug mode) leaning towards potentially an uninitialized variable. This seems to be coming from this PR's changes, as I can run the test in develop branch OK. |
Ok, I'll give it a try. |
I'm going to skip this test on Acorn. We'll have to look into why Acorn specifically. |
We can continue to work the issue #1944 , but I think we start the merging process here. |
Acorn failing tests:
|
@BrianCurtis-NOAA should these cases be turned off for Acorn? |
@BrianCurtis-NOAA code modified in PR109 is not executed in those tests either. The two experiments executing the code are atmaero_control_p8_rad_micro and merra2_thompson. |
Yes, go ahead and skip Acorn. |
Ok, will do. The fv3atm sub-pr is ready for review. Fyi @jkbk2004 @BrianCurtis-NOAA |
@AnningCheng-NOAA FV3atm sub-pr has been merged. Please go ahead and revert the changed url for the .gitmodule as well as update the submodule pointer. FV3atm hash: eadb52f6953502d8f5fc6ee3d07b257571013345 |
Zach, done
…On Tue, Oct 17, 2023 at 4:02 PM zach1221 ***@***.***> wrote:
@AnningCheng-NOAA <https://github.com/AnningCheng-NOAA> FV3atm sub-pr has
been merged. Please go ahead and revert the changed url for the .gitmodule
as well as update the submodule pointer. FV3atm hash:
eadb52f6953502d8f5fc6ee3d07b257571013345
—
Reply to this email directly, view it on GitHub
<#1915 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIMGYOGHSN3LW3PD573X73P5NAVCNFSM6AAAAAA5CBYAX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRXGA4DGOJZHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@jkbk2004 Please see if you can reproduce this issue NOAA-EMC/UPP#804 |
I am testing a PR branch which should have no impact on baselines, and all the Post files (ie, GFSPRS.GrbF00) are not reproducing. I will run some tests from develop, but I think there is an issue w/ the UPP update from yesterday. Partial list:
|
I think I've gotten everything aligned now (finally) and these tests are now reproducing. |
PR Author Checklist:
I have linked PR's from all sub-components involved in section below.
Bug fix for MERRA2 coupled Thompson microphysics and UPP update #1915
bug fixed for mraerosol NOAA-EMC/fv3atm#700
passing nc back from microphysics ccpp-physics#109
I am confirming reviews are completed in ALL sub-component PR's.
I have run the full RT suite on either Hera/Cheyenne AND have attached the log to this PR below this line:
I have added the list of all failed regression tests to "Anticipated changes" section.
I have filled out all sections of the template.
Description
Linked Issues and Pull Requests
Associated UFSWM Issue to close
related to issue 1914: #1914
Subcomponent Pull Requests
Blocking Dependencies
Subcomponents involved:
Anticipated Changes
Input data
Regression Tests:
baseline cases for mraerosol=T: merra2_thompson and atmaero_control_p8_rad_micro
Tests effected by changes in this PR:
Libraries
Code Managers Log
Testing Log: