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

Fix vertical indices of c1h and c2h in ideal case init of ph_1 #1280

Merged
merged 3 commits into from
Oct 9, 2020

Conversation

traupach
Copy link
Contributor

@traupach traupach commented Sep 8, 2020

TYPE: bug fix

KEYWORDS: ideal case, initialization

SOURCE: Tim Raupach (UNSW CCRC)

DESCRIPTION OF CHANGES:
Problem:
In ideal case initialization routines, incorrect indices were used in the calculation of ph_1 after a perturbation bubble was added.

Impact:
The calculation of perturbation geopotential ph_1 (perturbation geopotential) and Z_BASE (idealized base state height)
were affected.

  1. If hybrid coordinate option was selected, (hybrid_opt = 2), the entire ph_1 initialization is incorrect due to the offset
    in the indexing of c1h and c2h (half levels vs full levels).
  2. If hybrid_opt = 0, since the 1d variables are initialized kms:kme, there is no impact.

Solution:
The vertical indices were corrected in the calculations of ph_1 in the idealized initialization routine.

ISSUE:

LIST OF MODIFIED FILES:
M dyn_em/module_initialize_ideal.F

TESTS CONDUCTED:

  1. The jenkins testing is all pass.

RELEASE NOTE: In the ideal case initialization routines, incorrect indices were used in the calculation of ph_1 after a perturbation bubble was added. For the idealized cases that do not use the hybrid vertical coordinate by default, there is no impact at all. The ideal cases with a hill had the correct hybrid formulation.

TYPE: bug fix

KEYWORDS: ideal case, initialization

SOURCE: Tim Raupach (UNSW CCRC)

DESCRIPTION OF CHANGES:
Problem:
In ideal case initialization routines, incorrect indices were used in
the calculation of ph_1 after a perturbation bubble was added.

Solution:
The indices were corrected in the calculations of ph_1.

ISSUE:

LIST OF MODIFIED FILES:
M       dyn_em/module_initialize_ideal.F

TESTS CONDUCTED: None.

RELEASE NOTE:
Fixed incorrect vertical indices in ideal case initialization of ph_1.
Copy link
Collaborator

@dudhia dudhia left a comment

Choose a reason for hiding this comment

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

I agree with this change.

@dudhia
Copy link
Collaborator

dudhia commented Sep 8, 2020

My understanding is that this won't affect results unless the hybrid option is used which is off by default for these cases. Can the PR proposer confirm this and add some text in the PR on the impact? Thanks.

@traupach
Copy link
Contributor Author

traupach commented Sep 9, 2020

My understanding is that this won't affect results unless the hybrid option is used which is off by default for these cases. Can the PR proposer confirm this and add some text in the PR on the impact? Thanks.

Yes, hybrid_opt = 0 for these cases by default. When the hybrid option is off, only the top-most model level is affected by the incorrect indices. For vertical indices 1 to kde-1, c1h is set to 1 and c2h is set to 0 on lines 875 and 876 of module_initialize_ideal.F. However ph_1 is calculated for vertical indices k=2 to k=kte, and with the incorrect vertical indices c1h(kte) and c2h(kte) are used in the calculation of ph_1 for the top-most level (since kte == kde). c1h(kte) and c2h(kte) are unmodified by module_initialize_ideal.F and are both set to 0 (or possibly uninitialized?).

I've added a note about the impact in the PR.

@dudhia
Copy link
Collaborator

dudhia commented Sep 9, 2020

Thanks fro including the impact in the PR message. Looks fine to me.

@davegill davegill changed the base branch from master to release-v4.2.2 September 13, 2020 19:05
@davegill
Copy link
Contributor

@traupach
Tim,
What is the status of the jenkins testing? Did you receive an email? Can you post the text of that email in one of these comment boxes?

@davegill davegill changed the title Fix vertical indices in ideal case initialization calculation of ph_1. Fix vertical indices of ch1 in ideal case init of ph_1 Sep 13, 2020
@davegill davegill changed the title Fix vertical indices of ch1 in ideal case init of ph_1 Fix vertical indices of ch1 and ch2 in ideal case init of ph_1 Sep 13, 2020
@davegill davegill changed the title Fix vertical indices of ch1 and ch2 in ideal case init of ph_1 Fix vertical indices of c1h and c2h in ideal case init of ph_1 Sep 13, 2020
@davegill davegill self-requested a review October 7, 2020 14:54
@davegill
Copy link
Contributor

davegill commented Oct 7, 2020

Please find result of the WRF regression test cases in the attachment. This build is for Commit ID: 17f29b2, requested by: traupach for PR: #1280. For any query please send e-mail to David Gill.

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 19           18
Number of Builds       : 48           46
Number of Simulations  : 166           164        0
Number of Comparisons  : 105           104        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

Copy link
Collaborator

@dudhia dudhia left a comment

Choose a reason for hiding this comment

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

I agree with this change.

@davegill davegill merged commit e3456da into wrf-model:release-v4.2.2 Oct 9, 2020
@davegill
Copy link
Contributor

davegill commented Oct 9, 2020

@traupach
Tim,
Thanks for the contribution and finding the bugs! I apologize for the delay, but most of that was due to our broken testing system. We welcome you to continue your contributions back to the model for the benefit of the whole community.

You are now officially a WRF contributor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants