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

Revert w_crit_cfl back to 1.0 (from temporary 1.2) #1510

Merged
merged 1 commit into from
May 10, 2021

Conversation

davegill
Copy link
Contributor

@davegill davegill commented May 10, 2021

TYPE: bug fix

KEYWORDS: IEVA, cfl

SOURCE: internal

DESCRIPTION OF CHANGES:
This is a clean-up PR to 4412521 #1373 "Implicit Explicit Vertical Advection (IEVA)". We are resetting the default critical value to activate the w_damping option to the previous setting.

Problem:
The new namelist option w_crit_cfl replaces w_beta, where w_beta used to be set in module_model_constants.F and had a value of 1.0. Before this PR, the default value of w_crit_cfl was set to 1.2 in the Registry. If one didn't use the new namelist option to manually reset the value of w_crit_cfl to 1., that meant that w_damping would behave
differently from previous releases.

Solution:

  1. With consultation with the developer, the value for w_crit_cfl is now set to 1.0 in the Registry file. This gives similar and expected behavior for when w_damping kicks in.
  2. Also, a bit of column aligning in the neighborhood of this change was done to make the Registry a bit more tidy. "Try and leave this world a little better than you found it.", Robert Stephenson Smyth Baden-Powell.

LIST OF MODIFIED FILES:
modified: Registry/Registry.EM_COMMON

TESTS CONDUCTED:

  1. There are no problems to test, just resetting the critical value for activating w_damping (from 1.2 to 1.0).
  2. Let us all hope that Jenkins tests are all PASS.

modified:   Registry/Registry.EM_COMMON
@davegill davegill requested a review from a team as a code owner May 10, 2021 15:48
@davegill
Copy link
Contributor Author

@weiwangncar @louiswicker
Here is a massive mod. I understand it will take a few days to review this proposed change.

@louiswicker
Copy link
Contributor

louiswicker commented May 10, 2021 via email

@davegill
Copy link
Contributor Author

@weiwangncar
Would you please review this cfl change from 1.2 -> 1.0

@davegill
Copy link
Contributor Author

jenkins

Please find result of the WRF regression test cases in the attachment. This build is for Commit ID: e8a7578d60431a2eb046aa9ba6d4b0b2cf3cd4dc, requested by: davegill for PR: https://github.com/wrf-model/WRF/pull/1510. 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  : 163           161        0
    Number of Comparisons  : 103           102        0

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

@davegill davegill merged commit dc06ad1 into wrf-model:release-v4.3 May 10, 2021
vlakshmanan-scala pushed a commit to scala-computing/WRF that referenced this pull request Apr 4, 2024
TYPE: bug fix

KEYWORDS: IEVA, cfl

SOURCE: internal

DESCRIPTION OF CHANGES:
This is a clean-up PR to 4412521 wrf-model#1373 "Implicit Explicit Vertical Advection (IEVA)". We are resetting the 
default critical value to activate the w_damping option to the previous setting.

Problem:
The new namelist option `w_crit_cfl` replaces `w_beta`, where `w_beta` used to be set in module_model_constants.F and had a value of 1.0. Before this PR, the default value of `w_crit_cfl` was set to 1.2 in the Registry. If one didn't use the new namelist option to manually reset the value of `w_crit_cfl` to 1., that meant that w_damping would behave 
differently from previous releases.

Solution:
1. With consultation with the developer, the value for `w_crit_cfl` is now set to 1.0 in the Registry file. This gives similar and expected behavior for when w_damping kicks in.
2. Also, a bit of column aligning in the neighborhood of this change was done to make the Registry a bit more tidy. "Try and leave this world a little better than you found it.", Robert Stephenson Smyth Baden-Powell.

LIST OF MODIFIED FILES: 
modified:   Registry/Registry.EM_COMMON

TESTS CONDUCTED: 
1. There are no problems to test, just resetting the critical value for activating w_damping (from 1.2 to 1.0).
2. Let us all hope that Jenkins tests are all PASS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants