Skip to content

Updated FSBM (33-bins) scheme -- bug fix for release v4.2 #1135

Merged
davegill merged 7 commits intowrf-model:release-v4.2from
JS-WRF-SBM:FSBM-bug-fix-2
Mar 17, 2020
Merged

Updated FSBM (33-bins) scheme -- bug fix for release v4.2 #1135
davegill merged 7 commits intowrf-model:release-v4.2from
JS-WRF-SBM:FSBM-bug-fix-2

Conversation

@JS-WRF-SBM
Copy link
Contributor

@JS-WRF-SBM JS-WRF-SBM commented Mar 14, 2020

TYPE: bug fix

KEYWORDS: spectral-bin microphysics

SOURCE:
Jacob Shpund and Alexander Khain (The Hebrew University, Jerusalem, Israel)
kobby.shpund@mail.huji.ac.il

DESCRIPTION OF CHANGES:
The changes related to aerosol BC and for the FSBM scheme to be able to run using 33- or 43-bins.
The default version is 33-bins (see RELEASE NOTES for more info).

LIST OF MODIFIED FILES:
M phys/module_mp_fast_sbm.F

TESTS CONDUCTED:

  1. Compiled ideal / real cases using Intel compiler (ifort (IFORT) 19.1.0.166 20191121).
    A mesoscale convective system simulation was conducted by integrating for 10h the MC3E-0520
    test case. The simulation produced results as expected (see RELEASE NOTES for more info).
  2. Jenkins is all PASS

RELEASE NOTE:

  1. For simulating deep continental convective systems (as opposed to maritime convection), it is
    advised to use a 43-bins version of the updated FSBM scheme. In order to run the code using 43
    bins, one needs to modify:
  • The registry (to include more bins for the advection)
  • The corresponding loop indices inside the scheme, and the bin-space parameter (NKR) placed
    at the top of the main module
  • Switch between the 33-bins input tables with 43-bins
  1. More info about the updated scheme and corresponding results:
    Shpund, J., Khain, A., Lynn, B., Fan, J., Han, B., Ryzhkov, A., Snyder, J., Dudhia, J. and Gill, D., 2019. Simulating a Mesoscale Convective System Using WRF With a New Spectral Bin Microphysics: 1:
    Hail vs Graupel. Journal of Geophysical Research: Atmospheres.
    https://doi.org/10.1029/2019JD030576

  2. The 43 lookup tables can be downloaded here:
    https://drive.google.com/drive/folders/1qxYyQwKI1wKQYasDUkQvVgHs11prLiqA?usp=sharing

@davegill davegill changed the base branch from master to release-v4.2 March 14, 2020 03:38
@davegill
Copy link
Contributor

@JS-WRF-SBM @weiwangncar @dudhia
Kobby,
Would you please include the text from the jenkins email in this lower set of comments.

@JS-WRF-SBM
Copy link
Contributor Author

@davegill

Dave, the jenkins email was sent prior to the minor fix, for which the code didn't compile correctely.
Can you generate a new test? or should I nevertheless include the original one?

@davegill
Copy link
Contributor

@JS-WRF-SBM

Can you generate a new test?

Kobby,
Just modify your local copy, then

git add phys/module_mp_fast_sbm.F
git commit
git push -u origin FSM-bug-fix-2

In 90 minutes the test should complete again

@JS-WRF-SBM
Copy link
Contributor Author

git push -u origin FSM-bug-fix-2

@davegill
Can't push into origin >> permission denied.
Pushed to here. Not sure if this can generate a new Jenkins test.

@JS-WRF-SBM
Copy link
Contributor Author

@davegill
wrf_output.zip

This build is for Commit ID: 44461ec, requested by: JS-WRF-SBM for PR: #1135

Test Type | Expected | Received | Failed
= = = = = = = = = = = = = = = = = = = = = = = = = = = =
Number of Tests : 10 10
Number of Builds : 23 23
Number of Simulations : 143 143 125
Number of Comparisons : 91 10 0

The zip file is attached

@JS-WRF-SBM
Copy link
Contributor Author

@davegill

In Registry/registry.sbm, the vars th_old,qv_old,tempc,height for package fast_khain_lynn_shpund are in the scalar array.
This a bug from previous versions which I overlooked, since (at least) qv_old and th_old shouldn't be advected. These are the current state vars before advection which we use for tendencies.

Does the state vars at the package level in Registry/registry.sbm are advected?
If not, the above vars needs to be define as "state" NOT "scalar".
Pls. acknowledge and I will correct this.

@JS-WRF-SBM
Copy link
Contributor Author

In Registry/registry.sbm, the vars th_old,qv_old,tempc,height for package fast_khain_lynn_shpund are in the scalar array.
This a bug from previous versions which I overlooked, since (at least) qv_old and th_old shouldn't be advected. These are the current state vars before advection which we use for tendencies.

Does the state vars at the package level in Registry/registry.sbm are advected?
If not, the above vars needs to be define as "state" NOT "scalar".
Pls. acknowledge and I will correct this.

False alarm for the FAST-sbm, but for the FULL-sbm this is a bug (will not correct this now).

@davegill
Copy link
Contributor

@JS-WRF-SBM @weiwangncar @dudhia

Test Type | Expected | Received | Failed
= = = = = = = = = = = = = = = = = = = = = = = = = = = =
Number of Tests : 10 10
Number of Builds : 23 23
Number of Simulations : 143 143 125
Number of Comparisons : 91 10 0

This says that 125 tests failed. Something is not right.

@JS-WRF-SBM
Copy link
Contributor Author

@davegill @weiwangncar @dudhia

This says that 125 tests failed. Something is not right.

Dave,
From my end ideal and real builds as expected.
The odd thing is that after ./clean and ./configure the cpp flag stays "1":
-DBUILD_SBM_FAST=1

@davegill
Copy link
Contributor

@JS-WRF-SBM

The odd thing is that after ./clean and ./configure the cpp flag stays "1":

Kobby,
Yes, the code has FSBM built by default.

The failures could be the run or the build. That information is in the attached file with the email.

@davegill
Copy link
Contributor

@JS-WRF-SBM
Kobby,
Again, show the email with the jenkins test results after this commit.

@JS-WRF-SBM
Copy link
Contributor Author

@davegill

Kobby,
Again, show the email with the jenkins test results after this commit.

Waiting for the Jenkins per last commit (cbbab8a).

@JS-WRF-SBM
Copy link
Contributor Author

@davegill

Please find result of the WRF regression test cases in the attachment. This build is for Commit ID: cbbab8a, requested by: JS-WRF-SBM for PR: #1135.

Test Type              | Expected  | Received |  Failed
  = = = = = = = = = = = = = = = = = = = = = = = =  = = = =
  Number of Tests        : 10           10
  Number of Builds       : 23           23
  Number of Simulations  : 143           143         125
  Number of Comparisons  : 91           10         0

The zip file is attached.
Please let me know how can I help.
wrf_output_PR1135_cbbab8a.zip

@davegill
Copy link
Contributor

@kobby,
Every EM case is failing, all 125 of them. That usually means a compile error. I looked in the file you sent:

Screen Shot 2020-03-17 at 11 14 07 AM

Your declarations can't use the same name as those available from the WRF model via the Registry. Locally rename those values.

@JS-WRF-SBM
Copy link
Contributor Author

@davegill

Your declarations can't use the same name as those available from the WRF model via the Registry. Locally rename those values.

OK, but these are private to the fast_sbm scheme (it was fine for previous submissions).
Let me just ask the following: the only relevant change is at the top of the fast_sbm routine:
USE module_domain
USE module_dm

Which one of theseUSEstatement holds the (defined(DM_PARALLEL)) ?
I need it for the EM 3D cases for aerosols BC (around line #4303).
Maybe this is what causing the problem?

@davegill
Copy link
Contributor

@JS-WRF-SBM

Which one of these USE statements holds the (defined(DM_PARALLEL)) ?

Kobby,
The cpp directive [(defined(DM_PARALLEL))] does not come from a module via USE association. That directive is one of the compiler arguments.

@davegill
Copy link
Contributor

@JS-WRF-SBM
Kobby,
Let us know how that mod works for you.

@JS-WRF-SBM
Copy link
Contributor Author

@davegill

Let us know how that mod works for you.

Indeed, the aerosol BC are intact even after the removal of the USE statements.
Waiting for the automatic Jenkins test.

Thanks!

@davegill
Copy link
Contributor

@JS-WRF-SBM
Kobby,
🤞

@JS-WRF-SBM
Copy link
Contributor Author

JS-WRF-SBM commented Mar 17, 2020

@davegill

Kobby,
crossed_fingers

Please find result of the WRF regression test cases in the attachment. This build is for Commit ID: 56348c4, requested by: JS-WRF-SBM for PR: #1135.

Test Type              | Expected  | Received |  Failed
    = = = = = = = = = = = = = = = = = = = = = = = =  = = = =
    Number of Tests        : 10           10
    Number of Builds       : 23           23
    Number of Simulations  : 143           143        0
    Number of Comparisons  : 91           91        0
Failed Simulations are: 
    None
    Which comparisons are not bit-for-bit: 
    None

NOTE: if the ~WRF's god crosses figures, it means that someone messed up in a dum way.
Thanks !

@davegill
Copy link
Contributor

@JS-WRF-SBM
Kobby,
Great news!

Copy link
Contributor

@davegill davegill left a comment

Choose a reason for hiding this comment

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

Approved

@weiwangncar
Copy link
Collaborator

@JS-WRF Yes, please. Any information you can provide will be helpful.

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.

4 participants