-
Notifications
You must be signed in to change notification settings - Fork 118
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
ufs-release/public-v2: cherry-pick add GFSv16 dzmin change and changes for Dev/jcsda #63
ufs-release/public-v2: cherry-pick add GFSv16 dzmin change and changes for Dev/jcsda #63
Conversation
* add GFSv16 dzmin change * Add code changes in external_ic.F90 and fv_grid_tools.F90 for dev/jcsda, dycore PR #35 Co-authored-by: Jun Wang <[email protected]> Co-authored-by: Dan Holdaway <[email protected]>
I am not sure why those changes are picked for release v2. My understanding is that the dzmin is a temporary fix for GFSv16, and the release v2 is for regional application, is there a stability issue in regional application? The two other code changes are for ongoing JEDI development, since more changes are coming and may be required for DA, I'd suggest people to go to dev/emc branch if they want to try DA application, rather than the release branch. |
I think we need to ask @JeffBeck-NOAA and @jwolff-ncar, or at least they know whom to contact regarding the motivation for this PR. |
Hi Jun - Jacob Carley has been running his real-time parallel regional model with this change (dz_min) and finds the model is more stable. I cherry-picked the combined commit that included the jcsda changes, to simplify the git commit history. These are not required for the release branch, but were committed to develop together. |
This cherry-picked addition to the release branch was at the request of @JacobCarley-NOAA based on his experience with regional runs. |
@junwang-noaa @laurenchilutti - can you guys provide a formal review. |
If the reason for the updates is satisfactory to everyone, I can finish up the merge once I have formal reviews. |
Hi, all. The fix is OK for now, but an issue needs to be opened indicating
the need for a long-term, more fundamental fix, noting the objections that @xi
Chen - NOAA Affiliate <[email protected]> and I have raised. The solution
lies in changes to the topography filtering for certain small islands or
peninsulas, and/or in fixes to the physics: previously the GWD was causing
crashes for us, and more recently @kai-yuan Cheng - NOAA Affiliate
<[email protected]> has found strange behavior in the deep convection
scheme causing a similar crash.
Lucas
…On Mon, Jan 11, 2021 at 1:16 PM bensonr ***@***.***> wrote:
If the reason for the updates is satisfactory to everyone, I can finish up
the merge once I have formal reviews.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#63 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVDWUUDKTQ5RXI46LVDSZM57JANCNFSM4VZIV6SQ>
.
|
Upon further review, the change to dz_min is fine but I have severe
concerns about changing gz(km+1), which is the surface topography height.
This does not change phis, the saved, static topography (to the best of my
knowledge) but it could yield inconsistencies and noisy solutions. In
particular when running at convective scales someone needs to check closely
the behavior of w close to topography, especially steep marginally-resolved
topography.
Lucas
On Mon, Jan 11, 2021 at 1:32 PM Lucas Harris - NOAA Federal <
[email protected]> wrote:
… Hi, all. The fix is OK for now, but an issue needs to be opened indicating
the need for a long-term, more fundamental fix, noting the objections that @xi
Chen - NOAA Affiliate ***@***.***> and I have raised. The solution
lies in changes to the topography filtering for certain small islands or
peninsulas, and/or in fixes to the physics: previously the GWD was causing
crashes for us, and more recently @kai-yuan Cheng - NOAA Affiliate
***@***.***> has found strange behavior in the deep
convection scheme causing a similar crash.
Lucas
On Mon, Jan 11, 2021 at 1:16 PM bensonr ***@***.***> wrote:
> If the reason for the updates is satisfactory to everyone, I can finish
> up the merge once I have formal reviews.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#63 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AMUQRVDWUUDKTQ5RXI46LVDSZM57JANCNFSM4VZIV6SQ>
> .
>
|
Also what is the rationale for the changes in fv_grid_tools.F90 ?
On Mon, Jan 11, 2021 at 1:36 PM Lucas Harris - NOAA Federal <
[email protected]> wrote:
… Upon further review, the change to dz_min is fine but I have severe
concerns about changing gz(km+1), which is the surface topography height.
This does not change phis, the saved, static topography (to the best of my
knowledge) but it could yield inconsistencies and noisy solutions. In
particular when running at convective scales someone needs to check closely
the behavior of w close to topography, especially steep marginally-resolved
topography.
Lucas
On Mon, Jan 11, 2021 at 1:32 PM Lucas Harris - NOAA Federal <
***@***.***> wrote:
> Hi, all. The fix is OK for now, but an issue needs to be opened
> indicating the need for a long-term, more fundamental fix, noting the
> objections that @xi Chen - NOAA Affiliate ***@***.***> and I have
> raised. The solution lies in changes to the topography filtering for
> certain small islands or peninsulas, and/or in fixes to the physics:
> previously the GWD was causing crashes for us, and more recently @kai-yuan
> Cheng - NOAA Affiliate ***@***.***> has found strange
> behavior in the deep convection scheme causing a similar crash.
>
> Lucas
>
> On Mon, Jan 11, 2021 at 1:16 PM bensonr ***@***.***> wrote:
>
>> If the reason for the updates is satisfactory to everyone, I can finish
>> up the merge once I have formal reviews.
>>
>> —
>> You are receiving this because you are subscribed to this thread.
>> Reply to this email directly, view it on GitHub
>> <#63 (comment)>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AMUQRVDWUUDKTQ5RXI46LVDSZM57JANCNFSM4VZIV6SQ>
>> .
>>
>
|
Lucas, the changes in fv_grid_tools.F90 are required for DA, please see issue
#32 <NOAA-EMC#32>
…On Mon, Jan 11, 2021 at 1:38 PM lharris4 ***@***.***> wrote:
Also what is the rationale for the changes in fv_grid_tools.F90 ?
On Mon, Jan 11, 2021 at 1:36 PM Lucas Harris - NOAA Federal <
***@***.***> wrote:
> Upon further review, the change to dz_min is fine but I have severe
> concerns about changing gz(km+1), which is the surface topography height.
> This does not change phis, the saved, static topography (to the best of
my
> knowledge) but it could yield inconsistencies and noisy solutions. In
> particular when running at convective scales someone needs to check
closely
> the behavior of w close to topography, especially steep
marginally-resolved
> topography.
>
> Lucas
>
>
>
> On Mon, Jan 11, 2021 at 1:32 PM Lucas Harris - NOAA Federal <
> ***@***.***> wrote:
>
>> Hi, all. The fix is OK for now, but an issue needs to be opened
>> indicating the need for a long-term, more fundamental fix, noting the
>> objections that @xi Chen - NOAA Affiliate ***@***.***> and I have
>> raised. The solution lies in changes to the topography filtering for
>> certain small islands or peninsulas, and/or in fixes to the physics:
>> previously the GWD was causing crashes for us, and more recently
@kai-yuan
>> Cheng - NOAA Affiliate ***@***.***> has found strange
>> behavior in the deep convection scheme causing a similar crash.
>>
>> Lucas
>>
>> On Mon, Jan 11, 2021 at 1:16 PM bensonr ***@***.***>
wrote:
>>
>>> If the reason for the updates is satisfactory to everyone, I can finish
>>> up the merge once I have formal reviews.
>>>
>>> —
>>> You are receiving this because you are subscribed to this thread.
>>> Reply to this email directly, view it on GitHub
>>> <
#63 (comment)
>,
>>> or unsubscribe
>>> <
https://github.com/notifications/unsubscribe-auth/AMUQRVDWUUDKTQ5RXI46LVDSZM57JANCNFSM4VZIV6SQ
>
>>> .
>>>
>>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#63 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI7D6TN3HP6M3FJATFNWYATSZNAQ3ANCNFSM4VZIV6SQ>
.
|
Hi, Jun. Thanks. This makes more sense now, and removing the hard-coded
filename is something I have wanted to do for a while now.
…On Mon, Jan 11, 2021 at 1:42 PM Jun Wang ***@***.***> wrote:
Lucas, the changes in fv_grid_tools.F90 are required for DA, please see
issue
#32 <NOAA-EMC#32>
On Mon, Jan 11, 2021 at 1:38 PM lharris4 ***@***.***> wrote:
> Also what is the rationale for the changes in fv_grid_tools.F90 ?
>
>
> On Mon, Jan 11, 2021 at 1:36 PM Lucas Harris - NOAA Federal <
> ***@***.***> wrote:
>
> > Upon further review, the change to dz_min is fine but I have severe
> > concerns about changing gz(km+1), which is the surface topography
height.
> > This does not change phis, the saved, static topography (to the best of
> my
> > knowledge) but it could yield inconsistencies and noisy solutions. In
> > particular when running at convective scales someone needs to check
> closely
> > the behavior of w close to topography, especially steep
> marginally-resolved
> > topography.
> >
> > Lucas
> >
> >
> >
> > On Mon, Jan 11, 2021 at 1:32 PM Lucas Harris - NOAA Federal <
> > ***@***.***> wrote:
> >
> >> Hi, all. The fix is OK for now, but an issue needs to be opened
> >> indicating the need for a long-term, more fundamental fix, noting the
> >> objections that @xi Chen - NOAA Affiliate ***@***.***> and I
have
> >> raised. The solution lies in changes to the topography filtering for
> >> certain small islands or peninsulas, and/or in fixes to the physics:
> >> previously the GWD was causing crashes for us, and more recently
> @kai-yuan
> >> Cheng - NOAA Affiliate ***@***.***> has found strange
> >> behavior in the deep convection scheme causing a similar crash.
> >>
> >> Lucas
> >>
> >> On Mon, Jan 11, 2021 at 1:16 PM bensonr ***@***.***>
> wrote:
> >>
> >>> If the reason for the updates is satisfactory to everyone, I can
finish
> >>> up the merge once I have formal reviews.
> >>>
> >>> —
> >>> You are receiving this because you are subscribed to this thread.
> >>> Reply to this email directly, view it on GitHub
> >>> <
>
#63 (comment)
> >,
> >>> or unsubscribe
> >>> <
>
https://github.com/notifications/unsubscribe-auth/AMUQRVDWUUDKTQ5RXI46LVDSZM57JANCNFSM4VZIV6SQ
> >
> >>> .
> >>>
> >>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <
#63 (comment)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AI7D6TN3HP6M3FJATFNWYATSZNAQ3ANCNFSM4VZIV6SQ
>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#63 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVHQULVGPEUTEYRGI2TSZNBAVANCNFSM4VZIV6SQ>
.
|
Kate Zhou (@XiaqiongZhou-NOAA, [email protected]) may know more about this. |
In the higher resolution regional runs, dz_min=6 is also a rare situation as in GFSv16. In addition, the regional group also found there were many extreme warm temperature warning in their runs as Kai-Yuan found. It can not be solved with the dz_min fix. They are related physics tendencies. It is a different issue that has to be investigated. |
All, have we come to a conclusion about this PR for the SRW App release? |
I'd refer to @JacobCarley-NOAA for approval as I am not sure if the changes are required to be included in public release-v2. |
I'm good with this is those concerns expressed by @lharris4 were covered by comments from @XiaqiongZhou-NOAA . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jacob confirmed the code changes are required for release public_v2.
Hi, all. Thanks. I think it would be good if an issue were opened up
regarding the temporary nature of this fix.
Lucas
…On Tue, Jan 12, 2021 at 3:16 PM Jun Wang ***@***.***> wrote:
***@***.**** approved this pull request.
Jacob confirmed the code changes are required for release public_v2.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#63 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVFQ34YAEE4WRYKARXDSZSU2TANCNFSM4VZIV6SQ>
.
|
I opened a new issue, and referenced the discussion in this PR.
…On Tue, Jan 12, 2021 at 1:35 PM lharris4 ***@***.***> wrote:
Hi, all. Thanks. I think it would be good if an issue were opened up
regarding the temporary nature of this fix.
Lucas
On Tue, Jan 12, 2021 at 3:16 PM Jun Wang ***@***.***> wrote:
> ***@***.**** approved this pull request.
>
> Jacob confirmed the code changes are required for release public_v2.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <
#63 (review)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AMUQRVFQ34YAEE4WRYKARXDSZSU2TANCNFSM4VZIV6SQ
>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#63 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB2OWIWKVFTCIK4ES2GLMP3SZSXAZANCNFSM4VZIV6SQ>
.
|
Thanks :-)
On Tue, Jan 12, 2021 at 3:58 PM Laurie Carson <[email protected]>
wrote:
… I opened a new issue, and referenced the discussion in this PR.
On Tue, Jan 12, 2021 at 1:35 PM lharris4 ***@***.***> wrote:
> Hi, all. Thanks. I think it would be good if an issue were opened up
> regarding the temporary nature of this fix.
>
> Lucas
>
> On Tue, Jan 12, 2021 at 3:16 PM Jun Wang ***@***.***>
wrote:
>
> > ***@***.**** approved this pull request.
> >
> > Jacob confirmed the code changes are required for release public_v2.
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub
> > <
>
#63 (review)
> >,
> > or unsubscribe
> > <
>
https://github.com/notifications/unsubscribe-auth/AMUQRVFQ34YAEE4WRYKARXDSZSU2TANCNFSM4VZIV6SQ
> >
> > .
> >
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <
#63 (comment)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AB2OWIWKVFTCIK4ES2GLMP3SZSXAZANCNFSM4VZIV6SQ
>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#63 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVDU5OGSPBQ5O76PEWTSZSZXLANCNFSM4VZIV6SQ>
.
|
…erturbations (NOAA-GFDL#239) * Update .gitmodules and submodule pointers for ccpp-framework and ccpp-physics for gsl/develop branch * RUC ice for gsl/develop (replaces NOAA-GFDL#54 and NOAA-GFDL#56) (NOAA-GFDL#60) Implementation of RUC LSM ice model in CCPP * Fix bug in gfsphysics/GFS_layer/GFS_typedefs.F90 from merge * Remove lsm_ruc_sfc_sice from suite FV3_GSD_v0_unified_ugwp_suite and update submodule pointer for ccpp-physics * Remove sfc_sice from ccpp/suites/suite_FV3_GSD_v0_unified_ugwp_suite.xml * Update gsl/develop from develop 2020/12/08 (NOAA-GFDL#61) * Fix for updating stochastic physics on separate time-step. (NOAA-GFDL#199) This bug fix allows the random patterns in the stochastic physics persist the for a period of time (defined as SKEBINT,SPPTINT, etc.) before calculating new patterns. The fix is to move the allocation of the saved variables into the init section of stochastic_physics_wrapper, and remove the deallocates in the run section. * Bug fixes in (1) running with frac_grid=T and GFDL MP and (2) restarting with frac_grid=T (NOAA-GFDL#204) * -- Pointing to Moorthi's modifications in ccpp/physics, which fixed the crash when running GFDL MP with frac_grid=T; -- Not setting fice to zero in order to leave lake ice untouched; -- Restart in the coupled model with the default physics is reproducible, if bad water temperature is only filtered at initial time; Co-authored-with: Shrinivas Moorthi <[email protected]> Co-authored-with: Denise Worthen <[email protected]> * Revert change to .gitmodules and update submodule pointer for ccpp-physics * Update submodule pointer for ccpp-physics - MYNN surface layer updates and bugfixes (NOAA-GFDL#63) * Land stochastic perturbations (wrapper PR for NOAA-GFDL#65) (NOAA-GFDL#68) * Move initialization of stochastic physics after the physics initialization in CCPP. * Add albedo variables to land perturbations with lndp_type=2 option. Change to accommodate soil perturbations with RUC LSM. * Max/min soil moisture variables are introduced via GFS_Control_type variables instead of through the use of namelist_soilveg*. This is a more flexible way for different LSMs. * Added pores and resid variables for max/min soil moisture to GFS_typedefs.f90. * Remove tracer_sanitizer from all suites and from CCPP prebuild config * Add namelist option to apply land surface perturbations at every time step, clean up stochastic_physics/stochastic_physics_wrapper.F90 * Stochastic land perturbations: add roughness length over land to the perturbed variables (NOAA-GFDL#70) * Added roughness length over land to the perturbed variables. * Bugfix in gfsphysics/GFS_layer/GFS_typedefs.F90: remove Diag%cldcov, in particular the reset call because the variable is not allocated * Update .gitmodules and submodule pointer for GFDL_atmos_cubed_sphere for code review and testing * Revert change to .gitmodules for ccpp-physics, update submodule pointer for ccpp-physics * Revert change to .gitmodules and update submodule pointer for GFDL_atmos_cubed_sphere Co-authored-by: DomHeinzeller <[email protected]> Co-authored-by: Phil Pegion <[email protected]> Co-authored-by: shansun6 <[email protected]> Co-authored-by: tanyasmirnova <[email protected]>
This reverts commit 81b9be0.
* Revert "revise external_ic.F90 and fv_nudge.F90 (#68)" This reverts commit 32b44d9. * Revert "fixing call to pmaxmin to no longer get a compilation error when compiling with GNU." This reverts commit 2aa049c. * Revert "Update external_ic.F90 and fv_nudge.F90 to use allocatable arrays (#65)" This reverts commit 44211c0. * Revert "update external_ic.F90 and fv_nudge.F90 (#63)" This reverts commit 81b9be0. * Revert "Fix OVERLOAD_R4 ifdef block as suggested by @junwang-noaa (#60)" This reverts commit 63a4603. * Revert "Update code to use 'constantsR4_mod' module (#59)" This reverts commit 7b8ee4c.
…0-rc.1_updates_v2 Additional updates for GCM v11.0.0-rc.1
NOAA-EMC#44
Related PRs:
fv3atm: NOAA-EMC/fv3atm#223
ufs-weather-model: ufs-community/ufs-weather-model#356