bug fix: initialization of 1000-h fuel moisture content, computation of the net fuel load

[tmgiannaros]

TYPE: bug fix

KEYWORDS: fuel moisture initialisation, net fuel load computation, WRF-Fire

SOURCE: Theodore M. Giannaros (National Observatory of Athens, Greece)

DESCRIPTION OF CHANGES:
Problem:
Incorrect indexing in the initialization of fuel moisture content in subroutine read_namelist_fire, in WRF file phys/module_fr_fire_phys.F.

Solution:
Changed fmc_gc_initial_value(3)=fmc_1000h to fmc_gc_initial_value(4)=fmc_1000h in subroutine read_namelist_fire, in WRF file phys/module_fr_fire_phys.F.

LIST OF MODIFIED FILES:
M phys/module_fr_fire_phys.F

TESTS CONDUCTED:

1. The correction applied resolved the erroneous initialization of both 1000h and 100h fuel moisture content. Prior to
   this correction, fmc_gc_initial_value(3)=fmc_1000h and no initialization was carried out for fmc_gc_initial_value(4).
   This resulted to erroneously setting fmc_gc_initial_value(3)=fmc_1000h and fmc_gc_initial_value(4)=0 (was
   left uninitialized). Following the correction, both variables are correctly set (fmc_gc_initial_value(3)=fmc_100h and
   fmc_gc_initial_value(4)=fmc_1000h).
2. Jenkins is all PASS.

RELEASE NOTE: Bug fix for fire module related to the initialization of fuel moisture content due to an incorrect indexing assignment from the namelist entries for fuel classes.

[davegill]

jenkins

Please find result of the WRF regression test cases in the attachment. This build is for Commit ID: 8d2c1ee7099463d4190c9f2ccfc60cb481d0efe8, requested by: tmgiannaros for PR: https://github.com/wrf-model/WRF/pull/1516. 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]

@tmgiannaros
Theodore,
First of all, thank you for actively participating in the WRF modeling system.

This is your first pull request (PR) for WRF. When the blank commit is started, there is a fixed format for the information from you. This allows us to search for keywords in the git log. Here is the format:

TYPE: choose one of [bug fix, enhancement, new feature, feature removed, no impact, text only]

KEYWORDS: 5 to 10 words related to commit, separated by commas

SOURCE: Either "developer's name (affiliation)" .XOR. "internal" for a WRF Dev committee member

DESCRIPTION OF CHANGES:
Problem:
Generally or specifically, what was wrong and needed to be addressed?

Solution:
What was down algorithmically and in the source code to address the problem?

ISSUE: For use when this PR closes an issue.
Fixes #123

LIST OF MODIFIED FILES: list of changed files (use `git diff --name-status master` to get formatted list)

TESTS CONDUCTED:
1. Do mods fix problem? How can that be demonstrated, and was that test conducted?
2. Are the Jenkins tests all passing?

RELEASE NOTE: Include a stand-alone message suitable for the inclusion in the minor and annual releases. A publication citation is appropriate.

There is no issue that is being resolved, so you may remove that section. We would like for you to fill in something for the testing that you did. For example, you mention something about 0.3% change. A figure that shows before vs after would be great, as part of the commit message. For the RELEASE NOTE, this can be a slightly re-worded description. This part needs to be standalone, as these few sentences will be used in a bulleted list for the next release (in a few months).

Please go back through the commit message (you can do this directly from the github web page for this PR #1516 as the easy method) and edit it to conform to our format.

For your first PR, there will be more than usual communication back and forth, just so that you are prepared for your next PRs.

Again, we very much appreciate the time you have spent to relay this bug fix back to the community.

[tmgiannaros]

@davegill

Dave,
I've just noticed that my second commit (about wn) is not a bug, but rather a change in the methods/equations. As this change is related to the theory behind the formulation of the fire spread model, I think it should be removed from the current pull request. Is it possible to remove the second commit? If yes, I will also update the original commit message, deleting the part about the false commit.

Best,
Theodore

[davegill]

@tmgiannaros
Theodore,
We use the "squash and merge" option with github for the WRF model repository. Just put the source code back to the original version. From the local sandbox of your changes, edit the following line:

wn = fuelload/(1 + st(k))       ! net fuel loading, lb/ft^2

1. git add phys/module_fr_fire_phys.F
2. git commit -m "some single line description for you, that will be temporary"

For clarity:
3. git remote add theo https://github.com/tmgiannaros/WRF
4. git push -u theo master

Everything else should be automatic for the PR. Remember to edit the commit message with the github web interface.

[davegill]

@tmgiannaros
Theodore,

1. Would you be able to provide some "before versus after" figures to show the impact of your modifications?
2. You mentioned that you wanted to revert one of your changes (the definition of wn). Algebraically, the values differ only by O(st(k)^2), so this is not that big of a change for small st(k).

[davegill]

@tmgiannaros

1. I reverted the "wn" calculation.
2. After reviewing the proposed modifications, "before vs after" figures are not necessary.

[davegill]

@pedro-jm @dudhia @weiwangncar
Pedro,
I think that this is a REALLY simple bug-fix. The fuel moisture content for the 100h class was overwritten by the data from the 1000h class, AND the 1000h fuel moisture content class was left uninitialized (essentially zero). This was due to an accident in the original coding where the namelist run-time data was input. Would you, or one of the WRF Fire team, please review this and let us know your opinion.

[davegill]

@tmgiannaros
Theodore,
You can see that I split the bug-fix from the computational mod. Would you please take the SMALL computational mod and generate another pull request. This time, use the "develop" branch for your starting point.

[tmgiannaros]

@tmgiannaros
Theodore,
You can see that I split the bug-fix from the computational mod. Would you please take the SMALL computational mod and generate another pull request. This time, use the "develop" branch for your starting point.

Dave, the small computational mod does not necessarily mean an update to the code. If this part of the code is to be updated, then the entire set of equations in WRF Fire code need to be updated as well for consistency. I have not yet started doing so, but I plan to do so in the future.

Could you technically guide me on what you need me to do?

[davegill]

@tmgiannaros
Theodore,
I have already turned this PR into the bug fix. You may use the release-v4.3.2 branch as a base and resubmit the wn modification (if you would like to do so). I am re-opening this PR, as it is valid and necessary.

[pedro-jm]

@davegill
I just saw your question. I agree this is a bug to fix and fixes the problem in the way you described (reproduced below).

@pedro-jm @dudhia @weiwangncar Pedro, I think that this is a REALLY simple bug-fix. The fuel moisture content for the 100h class was overwritten by the data from the 1000h class, AND the 1000h fuel moisture content class was left uninitialized (essentially zero). This was due to an accident in the original coding where the namelist run-time data was input. Would you, or one of the WRF Fire team, please review this and let us know your opinion.

[davegill]

@pedro-jm
Pedro,
As a "WRF Fire" representative, would you approve this PR please? During the lockdown, without door-to-door conversations, we have bumped up the number of required approvals to two.

[pedro-jm]

@davegill Yes, I approve this PR.
@tmgiannaros Thanks for fixing this Theodore.

[dudhia]

@pedro-jm You can formally approve by going to files changed and then review changes buttons.

…

On Wed, Nov 17, 2021 at 3:30 PM pedro-jm ***@***.***> wrote: @davegill <https://github.com/davegill> Yes, I approve this PR. @tmgiannaros <https://github.com/tmgiannaros> Thanks for fixing this Theodore. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1516 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIZ77CNMKNBWBMQZQHVB4LUMQUILANCNFSM44YIWVAQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[smileMchen]

This PR looks fie to me.