Skip to content

Prevent 0 index in GOCART settling array#1113

Merged
davegill merged 2 commits intowrf-model:release-v4.2from
jordanschnell:stacy1
Mar 25, 2020
Merged

Prevent 0 index in GOCART settling array#1113
davegill merged 2 commits intowrf-model:release-v4.2from
jordanschnell:stacy1

Conversation

@jordanschnell
Copy link
Copy Markdown
Contributor

@jordanschnell jordanschnell commented Feb 28, 2020

TYPE: bug fix

KEYWORDS: GOCART, settling

SOURCE: Stacy Walters (internal)

DESCRIPTION OF CHANGES:
Put calculation of aerosol settling (variable = transfer_to_below_level) in layers > 1
into IF/ELSE block that calculates gravitational settling for layer = 1.

LIST OF MODIFIED FILES:
M chem/module_gocart_settling.F

TEST CONDUCTED:
Jenkins shows all tests PASS

Compiled with debug option -check bounds
BEFORE FIX: 2 indexes are OOB for l==1, 3rd dim of delz (> MAX) and 3rd dim of airden (0)

rsl.out.0000
l/=1, calculting transfer_to_below_level = 3.153560196907218E-014
l/=1, calculting transfer_to_below_level = 1.014890436929303E-012
l/=1, calculting transfer_to_below_level = 2.118741307102124E-011
l==1, still calculating transfer_to_below_level =
= (temp_tcvd_wk1)((delz(i,j,l2)*airden(i,j,l))/(delz(i,j,l2+1)*airden(i,j,l-1)))
l==1, index of airden(i,j,l-1) = 1 1 0
and index of delz(i,j,l2+1) = 1 1 31
but size of 3rd dim of delz = 30
rsl.error.0000
forrtl: severe (408): fort: (2): Subscript #3 of the array DELZ has value 31 which is greater than the upper bound of 30

AFTER FIX:
l/=1, calculating transfer_to_below_level = 3.153560196907218E-014
l/=1, calculating transfer_to_below_level = 1.014890436929303E-012
l/=1, calculating transfer_to_below_level = 2.118741307102124E-011
l==1, not calculating transfer_to_below_level

TYPE: bug fix

KEYWORDS: GOCART, settling

SOURCE: Stacy Walters (internal)

DESCRIPTION OF CHANGES: Put calculation of aerosol settling (variable = transfer_to_below_level) in layers > 1 into IF/ELSE block that calculates gravitational settling for layer = 1.

LIST OF MODIFIED FILES:

M chem/module_gocart_settling.F

TEST CONDUCTED:
@jordanschnell jordanschnell requested a review from a team as a code owner February 28, 2020 21:06
@davegill
Copy link
Copy Markdown
Contributor

@jordanschnell
Jordan,
You will need to change the base branch of this from develop to release-4.2 next week. We are making branch release-v4.2 soon, and this PR belongs on that new branch.

@jordanschnell jordanschnell changed the base branch from develop to release-v4.2 March 2, 2020 19:04
@davegill davegill changed the base branch from release-v4.2 to develop March 3, 2020 00:49
@davegill
Copy link
Copy Markdown
Contributor

davegill commented Mar 3, 2020

@jordanschnell
Jordan,
The change of base caused a MASSIVE change in the file count. You want to change one file, not 137. I have reverted the code to be based from the develop branch.

@davegill
Copy link
Copy Markdown
Contributor

davegill commented Mar 3, 2020

@jordanschnell
Jordan,
For the testing information in the PR commit message, we would like two pieces of information.

  1. A positive result that indicates the change that you provided actually does what it is supposed to do. For this PR, that could be as simple as some sort of print out that indicates a before vs after regarding incorrect indexing.
  2. We would like you to indicate that the jenkins test was all PASS.

@davegill davegill changed the base branch from develop to release-v4.2 March 4, 2020 13:08
@kayeekayee kayeekayee self-assigned this Mar 20, 2020
@jordanschnell
Copy link
Copy Markdown
Contributor Author

@kayeekayee @ravanah Hey all, can one of you review this for the 4.2 release today? Thanks!

@kayeekayee
Copy link
Copy Markdown
Contributor

Approved.

@davegill davegill self-requested a review March 25, 2020 04:17
Copy link
Copy Markdown
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.

Ka Yee said OK

@davegill davegill merged commit f43ae67 into wrf-model:release-v4.2 Mar 25, 2020
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