Breaching shading fat - #234
Open
jaimecarrasco wants to merge 21 commits into
Open
jaimecarrasco wants to merge 21 commits into
jaimecarrasco wants to merge 21 commits into
Conversation
…d_type, substr out_of_range guard
…isture modes, river/breaching Scott & Burgan kernel reimplemented: upstream's initialize_coeff/rate_of_spread_s/ flame_length with hardcoded coefficient tables are replaced by a compact rothermel_s() (BehavePlus formulation) plus an sbTable() lookup, hence the net reduction in size. - Portugal (Fernandes 211-237) as a preset of the S&B kernel instead of a separate kernel; removes FuelModelPortugal - --fmc-shading now works on all kernels (was Kitral-only; S&B had it hidden as --moisture-mode spatial). Same flag, different physics: Kitral applies an empirical shade penalty, S&B/Portugal solve an energy balance that also dries exposed cells - --moisture-mode direct|scenario|ffmc|conditioning|spatial, with --moisture-scenario DkLm - --fch-mode and --lb-mode select Kitral formulations - --river-shp linear barriers (ReadShp.h, no GDAL), --breach-factor, --spot-factor - --active-front for simultaneous multi-cell ignition
FuelModelSpain -> ScottAndBurganKernel, FuelModelKitral -> KitralKernel, FuelModelFBP -> CanadianFBPKernel. The old names conflated a fuel classification with a country: FuelModelSpain is really the Scott & Burgan engine (US origin), which now also runs Portugal. Updated includes, header guards, all five makefiles, and the MSVC project files. FuelModelUtils keeps its name: it holds shared fuel utilities, not a kernel.
MSVC only defines _S_IFMT/_S_IFREG, not the POSIX S_ISREG macro, so the msbuild/vcpkg path would fail to compile. No-op on POSIX and MinGW, where S_ISREG already exists. Note on the earlier portability commit: dirent's d_type was never broken under MSVC, since vcpkg.json pulls the dirent port (1.25) which provides it. The failure was MinGW-only, as MinGW does not use vcpkg.
Missed in the rename commit; test/unit_tests/test_kitral.cpp still included FuelModelKitral.h, breaking the CI build.
KitralKernel.h declared l_to_b with the old 2-argument signature while the definition takes lb_mode. The main build hid this because the only caller lives in the same translation unit; any other TU including the header fails to link, which is what broke the test build. Tests: rate_of_spread_k and active_rate_of_spreadPL04 now take arguments*, so the fixture builds one and sets ROS10Factor (parseArgs applies that default in production, the struct does not). The slope and wind reference values changed: slope no longer enters through at->se (now dead code in rate_of_spread_k) but through data->ps, and wind and slope are combined as vectors (S&B/FARSITE style) to also produce at->raz. The no-slope baseline is unchanged at 1.586, but higher wind diverges ~1.5% (3.276 vs 3.324 at ws=50). Rather than re-anchoring the reference numbers to the new implementation -- which would silently void the regression check -- those two cases now assert the shared baseline plus monotonicity, with the change documented inline.
The engine used harvest terminology for what the CLI already called firebreaks (--FirebreakCells). Mechanical rename: HarvestedCells -> firebreakPlan (the plan, year -> cells), harvestCells -> firebreakCells (the runtime set), HarvestPlan -> FirebreakPlan, parseHarvestedDF -> parseFirebreakDF. No behaviour change; the CLI flag and the CSV format are untouched. Touches ReadCSV.* and Cells.*, which were unmodified until now.
… semantics Ignitions and active front can now come from shapefiles, with the same semantics as their CSV counterparts (one ignition point per year; the front is a set of seed cells): --ignition-shp <shp> Point/MultiPoint, one point per year --active-front-shp <shp> PolyLine/Polygon (rasterised) or Point/MultiPoint Barriers are unified. Rivers, roads and firebreaks now share one mechanic: the cell stops being fuel (statusCells=3) and is flagged as a barrier so breaching can jump it. Without breaching they stop the fire. Previously --river-shp only fed the breaching test, so river cells kept burning and the flag was nearly a no-op on burnable terrain; this is a behaviour change (vilo100 goes from 6050 to 2790). --river-shp / --road-shp / --firebreak-shp <shp> single file --rivers / --roads / --firebreaks read Rivers|Roads|Firebreaks/*.shp Folders are opt-in on purpose: auto-loading on mere presence would let a dropped shapefile change results silently and would make A/B scenarios impossible. Rasterisation depends on geometry: for polygons a cell is taken when its CENTRE falls inside, so a strip narrower than a cell no longer becomes a cell-wide wall; polylines have no interior, so the traversed cells are taken. Also fixed: firebreaks were missing from the breaching barrier test, so --breach-factor never applied to them -- the central use case (sizing firebreak width) could not be modelled. And crossings over one-cell-wide barriers were attributed as generic 'barrier' because the immediate downwind neighbour is evaluated outside the loop that set the type. Shapefile reader gains point/multipoint reading, header bbox extraction (used to warn when a layer does not overlap the raster, almost always a CRS mistake), folder listing and point-in-polygon.
vcpkg's dirent port implements POSIX dirent on top of Win32 and includes <windows.h>, which defines min and max as macros. That turns std::max(...) into std::(...) and broke every translation unit including ReadShp.h -- 22 errors in Cell2Fire.cpp, including pre-existing std::min calls unrelated to this work. MinGW is unaffected: its dirent.h is native and does not pull windows.h in. NOMINMAX is set both in the header (before the include) and in the project's preprocessor definitions, so the fix does not depend on include order across translation units.
WindowsTests.vcxproj treats warnings as errors and lacks the preprocessor definitions
the main project has, so two problems surfaced only there:
- ReadArgs.h declared the args struct as an anonymous 'typedef struct {...} arguments;'.
With default member initialisers that triggers C5208, a warning in Cell2Fire.vcxproj
but an error here. Declared as a named 'struct arguments' instead; no behaviour
change, and it also silences the -Wnon-c-typedef-for-linkage warning gcc emits on
every translation unit.
- M_PI was undeclared because _USE_MATH_DEFINES was set in Cell2Fire.vcxproj only.
Added it, along with NOMINMAX, to all four WindowsTests configurations.
The vcxproj diff is inflated by a CRLF-to-LF normalisation; the substantive change is
five lines.
The reference outputs encoded upstream behaviour. Verified that unmodified upstream reproduces them exactly, so the differences are attributable to this branch: - fbp: same file count, 971 of 1245 files differ. The FBP path used the WD column directly as the internal azimuth while every other path adds 180, so fire propagated opposite to the documented convention (WD is the direction wind blows FROM). Fixed earlier in this branch; the targets still encoded the old direction. - sb: 753 -> 902 files. Scott & Burgan kernel reimplementation. - portugal: 837 -> 906 files. The Fernandes preset over the S&B kernel replaces the dedicated FuelModelPortugal kernel. - kitral: byte-identical, confirming the rate-of-spread reformulation does not alter these cases.
The comparison already stripped the version line from the logs; libtiff warnings needed the same treatment. Linux's libtiff emits 'Unknown field with tag' for GeoTIFF tags that the vcpkg build used by the Windows CI does not, so the -tif logs differed for reasons unrelated to fire behaviour and the targets were not portable across platforms. The test.ps1 diff is inflated by a CRLF-to-LF normalisation; the substantive change is one line.
EL PORTILLO, San Fernando (Colchagua), ignited 27-01-2021 14:52, controlled 31-01, 866 ha. Extends data/Kitral/Portillo-asc with slope/aspect derived from the DEM by Horn's method, the Rio Claro as a vector layer, and the observed perimeter, so the instance can be used for validation and not only as a demo. The fuel raster marked a 90 m median non-fuel strip along the channel, three to nine times the surveyed 10-30 m width. Those 1733 cells were restored to fuel (assigned the majority type of their burnable neighbourhood) and the river is supplied instead as a variable-width polygon in Rivers/, so the barrier is explicit and its width is the real one. fuels_original.asc is kept for traceability. River geometry from IDE Chile's 1:25,000 hydrographic network (CC-BY), clipped and reprojected to UTM 19S. See the README for the weather interpretation and for validation figures against the observed scar.
[... el mensaje largo que te pasé ...]
The targets were generated on Linux, whose libtiff emits 'Unknown field with tag' warnings for GeoTIFF tags that the vcpkg build used by the Windows CI does not. The script normalises the logs it produces but not the reference, so the warnings had to be removed from the targets themselves.
…dels Adds --fuel-adjustment <csv>, a per-fuel-type multiplier of the rate of spread in the manner of FARSITE's fuel adjustment factor: uniform in direction, applied to every cell of that fuel. Applied in Cells.cpp, so it works for all kernels without kernel-specific code. Applying the same factor to every fuel reproduces --EFactor exactly; the point of the new flag is the granularity, since a single global factor cannot correct a model that over-predicts in grass and under-predicts in shrub. The CSV takes either the fuel model code (GR3, PCH1, C-1, FM1) or the numeric id. Codes are resolved against the lookup table of the kernel in use, so a file written for one kernel cannot be applied to another by mistake. An unknown code aborts with the list of valid ones rather than being silently ignored, which would leave the run uncalibrated without anyone noticing. Also adds Anderson's 13 fuel models (1-13), taken from firelab/behave with their loads converted from lb/ft2 to tons/acre to match the rest of sbTable. Scott & Burgan (2005) supersedes them, but a great deal of existing cartography -- LANDFIRE among it -- is still coded that way, and the kernel could not read it at all. The table now covers 94 models: Anderson 13, Scott & Burgan 40, the behave extensions (SCAL, V-, M-, F-) 23, and Portugal 18. DataGenerator now prefers scott_and_burgan_lookup_table.csv, consistent with the kernel rename, falling back to spain_lookup_table.csv for existing instances.
Moisture is set with --moisture-mode/--moisture-scenario, which is explicit and does not depend on the weather file carrying an extra column. The column was parsed with std::stoi and aborted when it held a label rather than a number: data/ScottAndBurgan/ValeSousa-asc uses 'EWE' there and stopped working on this branch, while upstream handled it. A notice now points to the flag instead. Removing it changes no results. The regression instances that carry the column use FireScenario=2, whose derived moisture equals the defaults, and the Portugal path never reached that branch since --sim P sets its own moisture mode. Verified: sb-asc, sb-tif, portugal-asc and portugal-tif are byte-identical to the targets.
Adds --treatment-shp with --treat-cbh, --treat-cbd, --treat-ccf and --treat-fuel. Unlike a firebreak, a treated cell stays burnable: raising the canopy base height makes crown initiation harder, lowering bulk density makes crown propagation harder, and the surface fuel can be reassigned. There is deliberately no flag to lower the surface fuel load. In Scott & Burgan the load is part of what defines a model, together with bed depth, moisture of extinction and surface-area-to-volume ratio; lowering it alone would produce a fuel that is in no table and whose behaviour is calibrated against nothing. Reducing surface fuel is therefore expressed as a reassignment -- --treat-fuel TL3 states what is left after the treatment rather than how much was removed -- and the code is validated against the kernel's lookup table, so a typo aborts instead of being applied. Verified on Portillo, where crown fire is active: raising CBH to 6 m and lowering CBD to 0.02 reduce burnt cells from 8008 to 7238, with thinning contributing more than pruning, as expected. On vilo100, converting a stand to TL3 drops burnt area from 6050 to 3529 while converting it to SH7 raises it to 6322 -- a badly chosen treatment makes things worse, which is worth being able to show. The lookup-code resolution used by --fuel-adjustment was extracted into a shared helper, so both flags accept the fuel model code.
Two firebreak cells touching only at a vertex do not interrupt 8-connected spread: fire crosses the diagonal between them. Barriers given as geometry already avoided this because they block arcs, but barriers given per cell -- --FirebreakCells and the firebreak plan -- did not. On an 11x11 test with a complete anti-diagonal barrier drawn corner to corner, all 25 cells beyond it still burnt and the total dropped only by the 11 cells of the barrier itself. The two arcs crossing that vertex are now blocked whenever both diagonal cells belong to the plan, at a width no breaching or spotting factor can cross: a diagonal leak is not a gap the fire jumps according to its flame length, it is a passage that does not exist geometrically. With the fix the same barrier stops the fire completely (121 -> 55 cells burnt, 0 of 25 beyond it) and --breach-factor 5 does not reopen it. This matters well beyond the test case. In firebreak-placement experiments on a 2418-cell landscape, connected solutions were being perforated: their measured effectiveness rose from 17.0% to 40.9% reduction in burnt area once the leak was closed, while solutions made of isolated cells were unaffected, having no diagonal contacts to repair.
Two firebreak cells touching only at a vertex do not interrupt 8-connected spread: fire crosses the diagonal between them. Barriers given as geometry already avoided this because they block arcs, but barriers given per cell -- --FirebreakCells and the firebreak plan -- did not. On an 11x11 test with a complete anti-diagonal barrier drawn corner to corner, all 25 cells beyond it still burnt and the total dropped only by the 11 cells of the barrier itself. The two arcs crossing that vertex are now blocked whenever both diagonal cells belong to the plan, at a width no breaching or spotting factor can cross: a diagonal leak is not a gap the fire jumps according to its flame length, it is a passage that does not exist geometrically. With the fix the same barrier stops the fire completely (121 -> 55 cells burnt, 0 of 25 beyond it) and --breach-factor 5 does not reopen it. This matters well beyond the test case. In firebreak-placement experiments on a 2418-cell landscape, connected solutions were being perforated: their measured effectiveness rose from 17.0% to 40.9% reduction in burnt area once the leak was closed, while solutions made of isolated cells were unaffected, having no diagonal contacts to repair. Also fixes the Catch2 detection in the makefile. It probed for Catch2 v3 by linking a trivial program to /dev/null, which the Windows linker cannot write to: under MSYS2 the probe failed with 'final link failed: file truncated', the detection silently concluded Catch2 was absent, and the test target linked without it -- hundreds of unresolved symbols ending in an undefined reference to WinMain. The probe now links to a temporary file and removes it, so the unit tests can be run on Windows at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Portugal fuel models as a preset of the Scott & Burgan kernel, continuous dead-fuel
moisture modes, solar/topographic moisture conditioning unified across kernels, vector
(shapefile) inputs for ignitions and barriers, a single portable makefile for Linux,
macOS and Windows, and three Windows portability fixes.
Reference case
data/handson/vilo100(Scott & Burgan, ignition cell 5051, seed 1,--moisture-mode scenario --moisture-scenario D1L1) gives 6050 burnt cells onLinux (g++ 13), macOS Intel (Apple clang + libomp) and Windows (MSYS2 GCC 16) — no
floating-point divergence between toolchains.
Behaviour changes
Three, all intentional. Flagging them up front because they change results for existing
runs.
1. Portugal is now a preset of the S&B kernel, and
FuelModelPortugalis removed.--sim Pkeeps working but routes to Fernandes fuels (211–237) over the Scott & Burganengine instead of the dedicated kernel from #86/#191. Instances under
data/Portugal/will produce different numbers.
2. Rivers and roads now stop the fire. Previously
--river-shponly fed thebreaching test: river cells stayed burnable, so on burnable terrain the flag was nearly
a no-op (
vilo100gave 6050 with and without it). Rivers, roads and firebreaks nowshare one mechanic — the cell stops being fuel and is flagged as a barrier that
breaching can jump.
vilo100with a river across the leeward side goes from 6050 to2790.
3. The Kitral rate-of-spread formulation changed. Slope no longer enters through
at->se(a precomputed multiplier) but throughdata->ps(the slope raster, inpercent), and wind and slope are combined as vectors (S&B/FARSITE style) to also
produce
at->raz, the direction of maximum spread. The no-slope baseline is unchanged(1.586), but higher wind diverges ~1.5% (3.276 vs 3.324 at ws=50).
test/unit_tests/test_kitral.cppdetected this. Two cases were rewritten to assert thebaseline both formulations share plus monotonicity in wind and slope, rather than
re-anchoring the reference numbers to the new implementation — which would have
silently voided the regression check.
New features
Moisture
--moisture-mode direct|scenario|ffmc|conditioning|spatialfor Scott & Burgan andPortugal, with
--moisture-scenario DkLm(e.g.D1L1) for the scenario mode.--fmc-shading(solar/topographic conditioning) now works on all kernels. It wasKitral-only; in S&B the equivalent was hidden as the
spatialenum value, which staysas a backwards-compatible alias.
Both need a latitude. Note that
DataGenerator.cpp:693hardcodes 51.621244 (Banff,Alberta) when
--latitudeis absent — see Known issues.Kitral formulations
--fch-mode kitral|emc|ffmc|isi|buiand--lb-mode kitral|sb.Vector inputs
Ignitions and active front, with the same semantics as their CSV counterparts (one
ignition point per year; the front is a set of cells igniting simultaneously):
Barriers, single file or whole folder:
Folders are opt-in on purpose. Auto-loading on mere presence would let a dropped
shapefile change results silently, and would make A/B scenarios impossible.
Rasterisation depends on geometry: for polygons a cell is taken when its centre
falls inside, so a strip narrower than a cell no longer becomes a cell-wide wall; for
polylines (no interior) the traversed cells are taken. For sub-cell widths, prefer
polygons.
The shapefile reader (
ReadShp.h, no GDAL) gains point/multipoint reading, headerbounding-box extraction, folder listing and point-in-polygon.
Barriers and crossing
--river-shp,--road-shp,--breach-factor,--spot-factor.Fixes
Windows portability (3 real bugs). Found by cross-compiling with mingw-w64; none of
them shows up on Linux.
M_PIdoes not exist (not standard C++; MinGW and MSVC require it explicitly)-D_USE_MATH_DEFINESin the makefileint16_twithout<cstdint>(arrives transitively on Linux, not on Windows)dirent->d_type/DT_REGare a POSIX extension MinGW does not providestat()+S_ISREG, guarded for MSVCThe third uncovered a latent bug:
filename.substr(filename.size() - 4)throwsstd::out_of_rangefor names shorter than 4 characters. It never fired becaused_type == DT_REGfiltered out.and.., so removing that filter without a lengthguard would have crashed on any weather directory scan.
Note that
d_typewas never broken under MSVC —vcpkg.jsonpulls thedirentport(1.25), which provides it. The failure was MinGW-only.
Firebreaks were missing from the breaching barrier test, so
--breach-factorneverapplied to them and the central use case — sizing firebreak width so fire does not jump
it — could not be modelled.
Crossings over one-cell-wide barriers were attributed as generic
barrierbecausethe immediate downwind neighbour is evaluated outside the loop that set the type, so
RiverCrossings*.csvwas either mislabelled or not emitted at all.l_to_bdeclared with the old 2-argument signature while the definition takeslb_mode. The main build hid this because the only caller lives in the sametranslation unit; any other TU including the header fails to link, which is what broke
the unit-test build.
Shapefile CRS mismatch now warns. The
.shpheader bounding box (bytes 36–67) iscompared against the raster extent, so a layer in the wrong projection is reported
instead of silently contributing zero cells.
Renames
Kernels. The old names conflated a fuel classification with a country:
FuelModelSpainis really the Scott & Burgan engine (US origin), which now also runsPortugal.
FuelModelSpain.cpp/.hScottAndBurganKernel.cpp/.hFuelModelKitral.cpp/.hKitralKernel.cpp/.hFuelModelFBP.cpp/.hCanadianFBPKernel.cpp/.hFuelModelUtilskeeps its name: shared fuel utilities, not a kernel. Includes, headerguards, all five makefiles and the MSVC project files were updated. Done with
git mvso the renames are recorded as such and
git log --followkeeps working.Harvest vocabulary → firebreak. The engine used harvest terminology for what the
CLI already called firebreaks (
--FirebreakCells):HarvestedCells→firebreakPlan(the plan, year → cells),
harvestCells→firebreakCells(the runtime set),HarvestPlan→FirebreakPlan,parseHarvestedDF→parseFirebreakDF. No behaviourchange; the CLI flag and CSV format are untouched. This touches
ReadCSV.*andCells.*, which were unmodified until now.Build
One portable makefile replaces the platform-specific invocation. Auto-detects the OS;
the four variants (
makefile.khipu,.macos,.macos-static,.static) are keptbecause the CI workflows reference them.
macOS uses
/usr/bin/clang++by absolute path and adds-isysroot: a Homebrew LLVMclang++earlier inPATHfails with a cascade of SDK header errors. Windows linksstatically by default because QGIS does not have MSYS2 on
PATH, so a dynamic.exefails to start inside the plugin with an uninformative DLL error. The static library
list is taken from
makefile.static.Version is injected from
git describe --tags, falling back to a default.Validation
Cell2Fire.Ubuntu.noble.x86_64Cell2Fire_Darwin.macOS-13.i386-staticCell2Fire.exeReference cases on
vilo100, verified identically on Linux and Windows:--ignitions(CSV)--ignition-shp(same cell)--active-front(CSV, 11 cells)--active-front-shp(line or points)--river-shp(line across leeward side)--river-shp --breach-factor 5--river-shppolygon 6 m covering cell centres--river-shppolygon 6 m offset, covering noneThe last two are the point-in-centre rule: a narrow polygon blocks only where it
actually covers cell centres.
Moisture modes:
conditioning3501,conditioning --fmc-shading3466,--moisture-mode spatial3466 (alias equivalence). Breaching against a 40 mnon-burnable strip: 2790 without, 5639 at factor 5, 5888 at factor 20; factor 1 does
not cross, correctly, since reach ≤ 12 m.
Unit tests: 24 cases, 66 assertions, all passing.
Apple Silicon (arm64) is untested; the
macOS-14forced-version branch of the naminglogic is still theoretical.
Known issues, not fixed here
data->latis hardcoded to 51.621244 / −115.608378 (Banff, Alberta) inDataGenerator.cpp:693, inherited from the original Cell2Fire. Without--latitude,all solar geometry is computed for that latitude; for Chilean instances (~33°S) it is
the wrong hemisphere. Measured on
vilo100with daytime weather: ~4% on mean ROS, ~7%on max ROS. Mitigated on the FAT plugin side (latitude derived from the layer CRS), not
in the engine.
A missing
Weather.csvexits silently with code 0, printing only "Serial versionexecution". From the plugin this looks like a successful run with no results. It should
throw, as it does for a missing
fuels.asc.--scenarioand--moisture-scenarioare different flags with near-identicalnames. The former is the Portugal severity preset and does a raw
std::stoi, sopassing
"D1L1"to it aborts the process.--scenariono longer affects S&B physics —args->scenariosurvives only in averbose debug print (
ScottAndBurganKernel.cpp:543). Its only real use is the Portugalpreset.
ps = at->seis now dead code inrate_of_spread_k, left over from the previousslope path.
data/handson/vilo100/RUN_COMMANDS.mdis out of date: none of the four documentedcommands passes
--moisture-scenario, and thescenariomode does not read theFireScenariocolumn ofWeather.csv(that column is only a legacy fallback indirectmode). Without the flag the documented 6050 comes out as 3768.Notes for reviewers
Barriers are currently modelled as cells, but they are really edges in the
propagation graph: a 6 m road in 20 m cells becomes a cell-wide wall, and the breaching
width is quantised to
(s-1) * cellSide. The point-in-centre rule for polygons reducesthis but does not remove it. An edge-based model — detecting where the cell-to-cell
propagation vector intersects the barrier geometry, and measuring the actual crossed
width — is the natural next step and belongs in a separate branch. It needs a width per
feature, which a polyline does not carry (polygons do, or a
.dbfattribute would).makefile.khipucompiles without libtiff and encodes cluster-specific configuration;it and the other three variants were left in place because the CI references them.