-
Notifications
You must be signed in to change notification settings - Fork 320
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
bug in crop fire modeling #2566
Comments
Thanks for reporting this, @lifang0209. Can you post here what the effect of fixing the bug will be (option 1 above). Specifically, this statement is confusing "As a result, the calculated global total of burned area remains correct. In other words, this error mainly affects the fire component." What is it that's different? To clarify on your options above:
|
In general, it's better to submit many small PRs rather than one large PR. So if the fix for this can be easily split off from your main PR, that'd be preferable. Then if the new crop fire modeling and timing stuff can be split off into a different PR, that'd be great too. Thanks! |
If I have this right "option 3" is really #2550 and "option 2" is parts of those changes from #2550. Since, changing CNFireLi2021 would change answers in significant ways for clm5_0 (as well as clm6_0) I think we want to stick with just "option 1" there after we see how it affects results. The above seems like a small enough bug fix that it might be OK to change answers for clm5_0 as well as clm6_0. |
Yes, in terms of how we time the merges that makes sense. But after the bugfix, it would be great to see a PR focused just on other crop fire improvements, separate from a PR with the other planned improvements. |
cropfire_a1 was calibrated to match the observed and simulated global totals of crop fire burned area. Despite the incorrect additions of cropfire_a1/secsphr * fhd * fgdp * patch%wtcol(p) when fb=0, the overall impact on the total burned area was mitigated because cropfire_a1 was underestimated. The bug fix will correct these incorrect additions, ensuring that cropfire_a1 is more accurately represented.
Thank you for your clarification request. It helped me articulate exactly what I wanted to express. |
The option 1 doesn't work. In the file CNFireLi2021Mod.F90, the crop fuel load (fuelc_crop) is calculated only when the crop model is inactive. When the crop model is active, fuelc_crop is set to 0 in the existing code. Consequently, fb, which depends on fuelc_crop, always equals 0. If we apply only the fix suggested by option 1, there will be no fires on managed croplands, which isn't desirable. On the other hand, if I do not fix the bug, baf_crop would be incorrectly accumulated, leading to crop fires occur when crops are growing, which is unreasonable. |
In the CNFireLi2021Mod.F90, (1) fixed the bug described in ESCOMP#2566 to avoid incorrect accumulation of baf_crop and ensure that crop fires do not occur during the crop growing season; (2) confined crop fires to periods after harvest and before planting when crop module is active; (3) removed the dependency of baf_crop on fuel availability; (4) improved the modeling of the influence of socio-economic factors on crop burned area; (5) recalibrated the cropfire_a1 constant based on GFED5 crop burned area; (6) modify the declaration of CNFireArea in these F90 files to include the variable crop_inst, declare the variable crop_inst, and import and utilize crop_type from the module CropType. In addition, the modules CNDriverMod.F90, CNFireLi2014Mod.F90, CNFireLi2016Mod.F90, CNFireNoFireMod.F90, FATESFireBase.F90, and FireMethodType.F90 include the subroutine CNFireArea. (6) is implemented in these modules.
In the CNFireLi2024Mod.F90, (1) fixed the bug described in ESCOMP#2566 to avoid incorrect accumulation of baf_crop and ensure that crop fires do not occur during the crop growing season; (2) confined crop fires to periods after harvest and before planting when crop module is active; (3) removed the dependency of baf_crop on fuel availability; (4) improved the modeling of the influence of socio-economic factors on crop burned area; (5) recalibrated the cropfire_a1 constant based on GFED5 crop burned area; (6) modify the declaration of CNFireArea in these F90 files to include the variable crop_inst, declare the variable crop_inst, and import and utilize crop_type from the module CropType. In addition, the modules CNDriverMod.F90, CNFireLi2014Mod.F90, CNFireLi2016Mod.F90, CNFireLi2021Mod.F90, CNFireNoFireMod.F90, FATESFireBase.F90, and FireMethodType.F90 include the subroutine CNFireArea. (6) is implemented in these modules. (Original commit by Fang Li, [email protected].)
There is a bug in crop fires in CNFireLi2021Mod.F90 (the fire model developed for CLM5.1 and a possible default one in CESM3).
The bug causes baf_crop to be incorrectly accumulated, and the global constant cropfire_a1, calibrated using the inverse method, is significantly underestimated.
Currently, there are three options:
(1) Submit a PR to report the bug. I just conducted the BGC historical run and will obtain the corrected cropfire_a1 and submit the PR tomorrow afternoon.
(2) Submit a PR for the bug, along with developments in crop fire modeling and new crop fire timing inputs, sill in CNFireLi2021Mod (tomorrow afternoon).
(3) Submit the final code of all fire model development (i.e., CNFireLi2024Mod) on Sunday.
Which option do you prefer, and what are the rules for CLM regarding this?
cropfire_a1 was calibrated to match the observed and simulated global totals of crop fire burned area. Despite the incorrect additions of cropfire_a1/secsphr * fhd * fgdp * patch%wtcol(p) when fb=0 ( fhd * fgdp * patch%wtcol(p) always larger than 0 here), the overall impact on the total burned area was mitigated because cropfire_a1 was underestimated. The bug fix will correct these incorrect additions, ensuring that cropfire_a1 is more accurately represented.
The text was updated successfully, but these errors were encountered: