Add wild fire setting switches

[JoshuaRady]

This pull request adds several switches to the config file for granular control of wildfire behavior. The PR is being filed as a draft because changes were made on master before I finished work that overlap some functionality (see XX_dsb notes below). I would like some feedback on how to revise these code changes to fit with XX_dsb additions.

There are some comments and other know issues in the changes that will be cleaned up prior to the final pull request.

Purpose:

These changes have four main purposes.

1. Make it possible to turn fire on an off with settings rather than changing variable values in the code.
2. Make it possible to turn fire on and off for different run stages.
3. Make it possible to use Fire Return Interval (FRI) driven fire for transient and scenario stages.
4. Do some preparation for upcoming extensions to the wildfire implementation.

Details:

Fire Switches for Run Stages:

The config file settings fire_on_PR, fire_on_EQ, fire_on_SP, fire_on_TR, and fire_on_SC have been added. Each allows fire to be turned on for each of the corresponding run stages. Together these achieve purposes 1 and 2 above.

I have used values 0 for 'off' and 1 for 'on'. Currently there is no reason they could not be true and false, but I was leaving room for expansibility, as further mode information could be provided in the future. It is important to note that 'on' only means wildfire is possible, not that it will occur. Wildfire will occur only when the fire is specified to occur. The input files that govern fire have not been modified.

I have opted not to include a global fire switch that would override the above switches. Fire is turned off when all fire_on_XX switches are set to 0. A master switch could be convenient but I decided it would potentially lead to confusion. It could be added.

Outstanding Issues:

@hgenet says that fire does not occur in the pre-run stage. However, the code suggests otherwise so I'm not sure. If fire should not occur in the pre-run then fire_on_PR will be removed. As is there is no way to test if fire is running in the pre-run stage anyway. Feedback would be appreciated on this issue.

The XX_dsb settings that have been recently added partially duplicate the functionality of the fire_on_XX switches. Or at least they did. When I merged some changes a while back I had a conflict because these settings were being used in the fire logic. Now it seems those issues have gone away (actually I think the issue was in TEM.cpp, see Additional Technical Details)? If anyone knows the history here please weigh in. Fire is currently the only disturbance (dsb) but that may not always be the case.

Fire Mode Switch:

The config file setting fire_ignition_mode has been added. The default value of 0 makes the model run in the traditional way. FRI based fire is used for the equilibrium and spin-up stages and explicit fire for the transient and scenario stages.

Setting fire_ignition_mode = 1 causes FRI based fire to be used for the transient and scenario stages as well (purpose 3 above). The fri-fire.nc input file is used in the same manner as in the other run stages to control fire occurrence.

Work is ongoing on a revised wildfire implementation. I want to maintain the current/original/traditional wildfire behavior for backward compatibly and comparison purposes. New values for fire_ignition_mode will be added in the future to allow new functionality while preserving the existing.

The name fire_ignition_mode, while currently reasonably descriptive, might need to be renamed the governed fire behavior may extend beyond ignitions.

Namespacing:

The 'fire_' prefix has been used for all switches. It is likely that several additional wildfire related settings will be added in the future and I wanted to create a clear namespace for them. I placed the switches in the model_settings section of the config file because I thought that is where fit. This differs from the approach of the 'XX_eq' settings. Perhaps there could be a fire section? Any thoughts?

Model Behavior Changes:

In addition to the changes listed above there is one small change to fire behavior. Previously if a FRI = 0 value was specified for a grid cell that was simulated a divide by zero exception would occur and cause a crash. I have modified the logic so that FRI = 0 means no fire will occur for that grid cell. While the divide by zero is clearly a bug I suspect that is exists because FRI based fire has historically been been specified as non-zero for most simulations. It makes sense to me that 0 represents no fire return interval but others might disagree. This code change could be considered armoring of the code of a feature addition depending on your perspective. FRI = 0 can cause another problem in the equilibrium phase. See below.

Testing:

This code has been debugged and tested at 3057e2b. Simulations were performed with may different combinations of fire_on_XX and fire_ignition_mode settings. The switches were tested for the equilibrium, spin-up, and transient modes. The pre-run was not tested for the reasons discussed above. The scenario stage was also put off because the behavior is identical to the transient stage and because a new round of testing will be required after the issues discussed here are resolved. Fire was successfully turned on and off by stage for runs using identical fire input files. FRI based fire was confirmed for the transient stage. (See plots below.)

An issue came up in testing with variable FRI values. For spin-up and transient stages having different FRI lengths works fine. I encountered problems in the equilibrium stage however. As is documented in the code the spin-up needs to start before the end of a fire disturbance cycle. When the FRI lengths vary, and included a grid cell with FRI = 0, this caused a crash trying to recalculate the equilibrium run length. I have not investigated whether this is just because of the zero run length or the because of the variability. I'm not sure why this requirement is there so I didn't want to spend much time on it. This is not a issue created by my changes per se, but it makes a concise set of test simulations difficult.

Documentation:

Documentation is only inline currently. Documentation will be added to the documents when the issues discussed above are resolved.

[JoshuaRady]

Additional Technical Details:

Here is a summary of code changes made with this PR:

• Changes were made to the config file and locations in the code that import them.
• In TEM.cpp calls of the form runner.cohort.md->set_dsbmodule(runner.cohort.md->fire_on_XX); were added and the runner.cohort.md->set_dsbmodule(runner.cohort.md->XX_dsb); were commented out. What to do here needs to be resolved.
• We now pass in a pointer to the ModelData in WildFire::should_ignite().
• The logic in ModelData in WildFire::should_ignite() has been updated.
• Code in ModelData in WildFire::should_ignite() that would have been repeated with the modifications has been carved out into the new private function WildFire::isFireReturnDate().
• I found that the fri_derived variable WildFire::should_ignite() is not actually used and have removed it (it is actually commented out but will be removed).

[JoshuaRady]

Test Simulations:

Here are a selection of test simulations. I can post more if requested. All simulations were run for 4 grid cells from PR to TR stages.

Test Set 1:

In this set of simulations equilibrium, spin-up, and transient stages were run for 110 years. Fire was turned on for the spin-up and transient stages. FRI lengths of 0, 30, 40, and 50 years were used for the four grid cells. For explicit fire grid cells had 0, 1, 3, or 5 fires (on every 4th prime year). Severity varies with grid cell, but this is not really relevant. They plots show the effect of changing fire_ignition_mode from 0 to 1.

Test Set 2:

In this set of simulations equilibrium was run for the default 1000 years. The spin-up and transient stages were run for 210 years. FRI was set to 100 years for all grid cells. There is no evidence of fire in cell 1,1 because the severity = 0. Explicit fire setting were the same as above. Three plot showing the effect of fire_ignition_mode and the ability to control fire across run stages with the fire_on_XX switches are shown.

[hgenet]

Thanks Joshua for the very well documented request. I'll try to read it before the end of the week.

[tobeycarman]

re: the history of the conflict between XX_dsb and the fire_on_EQ flags is that your changes started before we merged PR #628, so they overlap.

re: turning fire on in pre-run, there is no use case for this but it would technically be possible to turn the flag on. The ecosystem would probably crash quickly. There is really no need to have the setting exposed for the pre-run stage, but it makes the files more consistent with everything else 🤷

re: FRI=0 being no fire, that makese sense. Probably that should get documented in a section describing the input files (maybe "Model Overview" > Inputs/Outputs" > "Inputs"?) https://uaf-arctic-eco-modeling.github.io/dvm-dos-tem/model_overview.html#inputs

re: model behavior changes (FRI = 0 in equlibrium phase), it could be a divide by zero problem or could be the problem described in issue #613.

re: as for what to do about merging this PR, got to talk with @hgenet and @rarutter about the overall design and are thinking that a config format to aim for would look like this:

"stage_settings": {
  "pre-run": {
    "dsl": false,
    "fire": false,
    ...
  },
  "eq": {
    "dsl": true,
    "fire": true,
    ...
  },
  // ... etc
},
"module_settings" {
  "dsb": {
    "fire": {
      // 0: default/old, 1: use fri for this stage, 2: future
      "ignition_pr": 
      "ignition_eq":
      "ignition_tr":
      "ignition_sp":
      "ignition_sc":

      // more future controls per staage can go here...
      "severity_pr":
      "severity_eq":
      // ...
    },
    "insect": { ... },
    "thermokarst":{ ... },
  }
},
"model_settings" : { }

Would this accomodate your anticipated needs?

We are not sure how many hoops to jump thru for merging this current PR, but we'd like to aim towards the design laid out above. One way to move forward would be to effect the entire refactor right now (more commits to this PR). Another would be to implement only parts (i.e. renaming the config stage_settings dsb --> fire and the ModelData.dsbmodule member), but not all the functions related to dsb at this time. This would leave the code in a slightly more confusing state (confusing names) but would be much less work upfront. If we go this route we would want to leave comments in the code explaing why the name dsb is used in some places and fire in others so that our future selves don't go crazy.

If time permits it might be better to fully flesh out this design and undertake the whole refactor now. If merging the PR sooner would greatly facilitate your work we could defer some of the refactoring and just button up a few small details.

[JoshuaRady]

Sorry for the long silence on this PR. I'm planning on putting some time into this in order to resolve the remaining issues over the next couple of weeks. I'm providing an item by item responses to @tobeycarman's comments below but in short I think the proposed config file compromise will work for me and I can work on implementing the revisions as part of this PR (with one question below). A Zoom discussion would probably be good to make sure I'm following everything said above. I've started pulling updates from master to my local repo and at some point those will be pushed back here with further edits. It might be a few commits before any further attention is needed. I yell when I get to another stable version.

Specific responses:

re: turning fire on in pre-run, there is no use case for this but it would technically be possible to turn the flag on...

I don't want to change behavior in the pre-run so I am removing the switch in the config file.

re: FRI=0 being no fire, that makese sense. Probably that should get documented in a section describing the input files...

I will work on updates to the documentation to cover this and tbe other changes (once finalized). Is is preferred to documentation changes as port of a code PR or as a separate one?

re: model behavior changes (FRI = 0 in equlibrium phase), it could be a divide by zero problem or could be the problem described in issue #613.

I'm familiar with issue #613 and this should be separate. My code fix seems to be working but we can reassess if any issues come up in subsequent testing.

re: as for what to do about merging this PR, got to talk with @hgenet and @rarutter about the overall design and are thinking that a config format to aim for would look like this:

I'm am fine with the solution proposed with separate switches to turn fire on and off for each run stage and dsb:fire switches to control fire run mode. ignition_XX is probably a fine terminology for the these switches. I anticipate additional fire mode switches may be needed fairly soon so this namespace will work nicely.

We are not sure how many hoops to jump thru...

I think I follow this but a Zoom discussion (per above) might help. I'm glad to help with the code changes if I'm sure I know what you want.

[JoshuaRady]

As I've been working a few questions have come up:

1. As asked and answered above fire should not be on in the pre-run stage. However, the current code does set up for FRI based fire in the pre-run, though I don't know if the downstream code will actually lead to fire effects. So is this an idiosyncrasy that should be revised or am I missing something? I'm not including a fire switch for the PR stage. I want to make sure everything is consistent.

2. I'm not sure what to do with the XX_dsb flag. Currently I'm feeding the fire_on_XX flags to runner.cohort.md->set_dsbmodule(), ignoring XX_dsb. Should I be passing in XX_dsb and then adding a further check either in runner.cohort.md->set_dsbmodule() to see is fire is on?

3. A note rather than a question: The suggestion above has ignition_XX flags for all run stages. This doesn't really make sense currently if fire should alway be off in the PR stage and is always FRI in the EQ and SP stages. Therefore I have only implemented them for the TR and SC stages.