WISE-Developers / WISE_FWI_Module

WISE_FWI_Module formerly known as FWI
GNU Affero General Public License v3.0
0 stars 0 forks source link

Logic behind imprecision in FFMC calculation #6

Open BadgerOnABike opened 2 months ago

BadgerOnABike commented 2 months ago

In an effort to determine what each of the systems calculating FWI are up to we noticed that FFMC had been calculated with seemingly random significant digits. The correct value is 147.27723 however, like the referenced code below we see precision being applied based on an hourly fraction rather than just using the most precise version. The rounded precision called is also different than what rounding this value would do.

https://github.com/WISE-Developers/WISE_FWI_Module/blob/c72467755c71a7915a144850339f447a180220fd/java/src/main/java/ca/wise/fwi/Fwi.java#L311-L314

This variety of error can be seen in a number of places, I've not documented them all yet, I'd just like clarification on why this is happening.

We have looked through the original C code and do not see these rounding structures.

RobBryce commented 2 months ago

This is by design and consistent with the C/C++ code which has been validated numerous times. The extra precision for sub-hourly calculations dates back to discussions with Cordy, Neal, Mike Wotton. Sub-hourly calculations need extra precision to properly converge on the hour. However, hourly calculations cannot have this precision to continue to produce values which match published results. We noted this when this was discovered. We all agreed that more precision should be applied at all cases, but we couldn't go an change the standard, or have Prometheus (at the time) produce outputs which weren't consistent with the standard.

For Java, we also found slight rounding differences from the C++ code, which is expected due to language spec's. I may have details on this, but it'll mean digging through some ancient emails.

BadgerOnABike commented 2 months ago

Alright, interesting, for what it's worth, that level of precision is now in the CFFDRS R and TS libraries. We will endeavor to have it in all the libraries to ensure consistent behaviours and to have rounding inconsistencies as far down as possible.

I've put a note in to Mike as well to get his take as we move toward next gen, but considering Jordan has been the primary programmer for that I think I'm safe in suggesting consistent precision will be used there. Likely to some nominal number of sig digs (~5 or something).

CFFDRS code repos - https://github.com/cffdrs

RobBryce commented 1 month ago

I reviewed the CFFDRS code. As noted by @BadgerOnABike above:

Requests:

  1. Please make the official code consistent with itself.
  2. If the code should have more precision than what was originally published, can you please provide a reference document for this change? The precision does have cumulative effects. The tweak to the WISE code was a non-standard increase in precision for non-standard sub-hourly calculations.
  3. The WISE code has additional checks on parameters (wind speed, temperature), which I ask that the official code considers adding, for further consistency. If that can be met, then the C code from WISE could be a 4th language in the CFFDRS code base.
BadgerOnABike commented 1 month ago

Totally appropriate requests.

  1. Jordan is about to push these to dev, so that's done.
  2. Definitely a worthwhile endeavor, it would be good to document why it was initially imprecise and why we are moving towards consistent precision as well as "anticipated rounding" significant digits.
  3. Can you link to the locations these additional checks are occurring in WISE. I'm all for additional checks where appropriate, I'm curious if this is something that needs to live where it is or if there is a more optimal insurance location.
RobBryce commented 1 month ago

In fwi.cpp, lines 193-210, the following logic is imposed. Similar logic exists for all other routines too. We don't report errors, but instead limit values to ranges in case any weather patches inadvertently exceed values - we still want to model the fire.

if ((in_ffmc < 0.0) || (in_ffmc > 101.0) ||
    (rain < 0.0) || (rain > 600.0))
    return -98;

if (temperature < -50.0)
    temperature = -50.0;
else if (temperature > 60.0)
    temperature = 60.0;

if (rh < 0.0)
    rh = 0.0;
else if (rh > 1.0)
    rh = 1.0;

if (ws > 200.0)
    ws = 200.0;
else if (ws < 0.0)
    ws = 0.0;](url)