Repository navigation
chore(release): bump version to v2.6.0 - #214
Merged
Merged
Conversation
Contributor
|
Hi @antonio-leblanc that woulb be great, but we have a problem for master from @3dfirelab, the bad constant in rothermel.. I'd like it in the new release, it means that the test cases should be re-generated with this new constant.... you think you can do that ? -- confirmed: sd is already in 1/ft there, and RothermelAndrews2018 and Farsite both use 3.59e-4. https://github.com/forefireAPI/forefire/pull/195 Other thing, FF is very downloaded, and i hit the git LFS.. you think this may be an occasion to not release binary files.. host them somewhere ? i can put them on zenodo...or on some server.. |
Collaborator
Author
filippi
approved these changes
Oct 27, 2026
Contributor
|
Moved to 2.6 ! thanks @antonio-leblanc |
filippi
added a commit
that referenced
this pull request
Oct 28, 2026
* fix(core): stop ~ForeFireModel freeing an uninitialised pointer
The constructor left `properties` uninitialised. Models that register at
least one property overwrite it in registerProperty, but the six that
register none never do -- Iso, heatFluxBasic, factorChemFlux,
CraterVaporFluxModel, LavaSO2Flux and vaporFluxBasic. Destroying one of
those passed whatever the member happened to be built over to delete[].
The same destructor also freed fuelPropertiesTable with delete[], while
DataBroker::extractFuelProperties allocates it with a scalar new.
Neither has been observed in a run, because FireDomain keeps its models
in propModelsTable and fluxModelsTable and frees neither -- so nothing
destroys a model today. Both become reachable as soon as anything does,
including a test.
* test(models): cover every propagation and flux model, build with -Wall -Wextra
tests/runff runs one case through one propagation model (Rothermel, set
in params.ff). Nothing was watching the other sixteen propagation models
or any of the sixteen flux models.
These tests exercise one model at a time, without a simulation.
getSpeedForNode splits in two -- the DataBroker gathers properties into
a double*, then the model does arithmetic on that array -- and only the
second half is physics. ModelSandbox builds an empty FireDomain purely
to instantiate models; Inputs fills their property array by property
name rather than by index, so reordering a constructor's registerProperty
calls cannot silently change what a test measures.
Two kinds of assertion, and the README says which to trust. Invariants
-- no spread without fuel, more wind never means less spread, a downslope
is not an upslope, released energy is independent of how the time window
is cut -- hold whatever the implementation is. Pinned rates of spread
merely record what ForeFire produces today, at 1e-5 relative tolerance so
that -march=native and FP contraction differences between runners do not
trip them.
Parameters are process-global, so the sandbox snapshots the parameter map
and restores it on destruction; without that, three of five
--order-by=rand seeds fail.
Also compiles ForeFire's own sources with -Wall -Wextra. The warnings are
not clean yet, so they are informational: FOREFIRE_WARNINGS_AS_ERRORS
turns them fatal for whoever is clearing a file, and
FOREFIRE_ENABLE_WARNINGS=OFF turns them off. NetCDF's headers are marked
SYSTEM so their own warnings stay out of the output.
The framework is doctest 2.5.3, vendored as a single header under
third_party/ so a build needs no network. Nothing there is compiled into
libforefireL. FOREFIRE_BUILD_TESTS defaults on, except for wheel builds.
* ci: run the native workflows on pull requests to dev
Both workflows only triggered on master, so nothing integrating into dev
was tested. main.yml already carried a commented-out dev push trigger.
* ci: take ownership of build/ before running ctest on Linux
The Linux workflow builds with `sudo bash ./install-forefire.sh -y`, so
build/ is root-owned and ctest cannot create build/Testing/Temporary as
the runner user. It exited 8 without running a single test.
macOS is unaffected: install-forefire-osx.sh runs unprivileged.
* fix(models): stop freeing the properties array twice
`properties` is allocated by each model's own constructor with
`new double[numProperties]` and freed by ~ForeFireModel. Sixteen flux
models and two propagation models freed it again in their own
destructor, so destroying any model that registers at least one property
was a double free. The flux ones also used scalar `delete` on an array
allocated with `new[]`.
Removing the derived deletes and leaving it to the base class is the
whole fix; no ownership changes.
This has never been hit in production because FireDomain keeps its
models in propModelsTable and fluxModelsTable and frees neither, so
nothing destroys a model today. It is reachable from anything that does,
including a test -- which is why the test that covers it comes with it:
`every model can be destroyed` now runs all 33 models rather than the
six that register no properties, and `destroying a model does not
disturb the next one` allocates across a destruction, since a double
free often surfaces as the next allocation coming back wrong rather than
as an immediate abort.
Verified by reintroducing a single delete: the suite aborts with
"double free or corruption (!prev)" and doctest reports the case as
CRASHED - SIGABRT.
* fix(propagation): keep ROS finite and damped at high dead moisture
Two rate-of-spread models misbehave once dead fuel moisture reaches or
exceeds the moisture of extinction. Neither is reachable today, because
Md is a static column of the fuel table that never gets that high, but
both become reachable the moment dead fuel moisture is driven by a
dynamic field.
RothermelAndrews2018 returned NaN. Its moisture damping cubic is only
defined below the moisture of extinction: it reaches zero at mf/me == 1
and turns negative past it, driving the reaction intensity negative,
then the wind limit `wv = 0.9 * RI` negative, then pow(wv, B) to NaN for
the non-integer B. The `R <= 0` exit test is false for NaN, so the value
escaped into FireNode::velocity. Return zero before the cubic is
evaluated, and negate the exit test so a NaN can no longer slip past it.
The Balbi family sped up instead of slowing down. xsi is the share of
combustion energy spent vaporising the moisture the live fuel carries in
excess of the dead fuel; when the dead fuel is the wetter of the two it
goes negative, so (1-xsi) exceeds 1 and amplifies both the radiant term
and the flame temperature, which R00 raises to the fourth power. At
Md=3.0 that put the flame temperature near 18000 K and made the fire
spread 33x faster than at Md=1.0. Floor xsi at zero: the dead fuel's own
penalty is already carried by the 1/(1 + a*Md) factor. Applied to all
five models that share the expression.
xsi is deliberately not capped above. With the current fuel tables it
reaches ~5.3, which makes the flame temperature negative, but that traces
back to Deltah equalling DeltaH in most rows of fuels.csv rather than to
the model, and capping would move every existing calibration.
Behaviour below the thresholds is unchanged to the last digit, and the
runff KML and NetCDF references still match.
* test(propagation): add dead fuel moisture invariant suite
The runff test compares ForeFire against frozen ForeFire output, so it
cannot tell a physics fix from a physics regression: any deliberate
change to a spread model breaks it, and any accidental one that leaves
real_case.ff untouched slips through. This suite holds no reference data.
Every assertion follows analytically from the published spread equations,
so it stays valid across recalibration and across the planned switch from
a static fuel.Md column to a dynamic dead-moisture field.
Five invariants, over Rothermel, RothermelAndrews2018 and BalbiNov2011:
finite ROS is a real number for every moisture in [0, 3]
monotonic ROS decreases as dead fuel moisture rises
extinction ROS reaches zero at Md >= me and stays there
responsive a dynamic dead-moisture layer reaches the model
resolved DataBroker finds an optimised getter for every property
`responsive` skips until dynamic dead moisture exists, then activates.
`resolved` exists because DataBroker does not raise on an unknown
property: it warns and silently falls back to an un-optimised path, so a
mistyped property name yields stale values rather than an error.
Two constraints the probe has to respect. Each probe runs in its own
interpreter, because the core keeps mutable global state and a second
ForeFire() in one process inherits the first one's parameters -- an
in-process sweep returns one identical displacement for every input.
And relax is forced to 1, because below that FireNode::update blends in
the ignition velocity, which decays geometrically but never reaches zero,
so an extinguished front still creeps and extinction is unobservable.
Each assertion was mutation-checked: flipping the sign of the moisture
damping term, applying the dead-moisture layer inverted, removing the
moisture-of-extinction cutoff and mistyping a property name each turn the
matching invariant red. Runs in about 16 seconds.
* fix(databroker): stop "moisture" being served the five-slot getter
registerPropagationModel matched the prefix "moist" and routed anything
starting with it to getMoisturesProperties, which fills five slots from
the moistures.* scalar parameters. That getter exists for the NFDRS-style
group FarsitePropagationModel registers as moist.ones / moist.tens /
moist.hundreds / moist.liveh / moist.livew, where five reserved slots
match five written values.
The prefix also caught the singular "moisture", which reserves one slot.
Balbi2015, Balbi2020 and BalbiNov2011TMdMl each register it, so every
property they registered afterwards was written four slots too late and
the last four ran past the end of the properties array. Balbi2020 read
its temperature out of moistures.liveh, its Rhod out of moistures.tens
and its Md out of moistures.hundreds, and wrote 28 doubles into a
24-element allocation. Measured, it spread the same 42.43 m -- the
distance covered by the initial front alone -- at every fuel-table Md
from 0.02 to 0.40: the model could not see its own fuel table.
Match "moist." instead, so the group still reaches the five-slot getter
while "moisture" resolves through propPropertiesGetters to getMoisture,
which writes exactly one value.
That leaves getMoisture reachable for those three models for the first
time, and it dereferenced moistureLayer unconditionally. Guard both
overloads and fall back to the moistures.ones parameter, which is the
value these models were already receiving, so no configured simulation
changes its numbers; warn once, since a model asking for a moisture
field and getting a constant is almost certainly misconfigured.
The runff KML and NetCDF references are unaffected: Rothermel registers
no moisture property.
* test(propagation): cover the models that register "moisture"
Balbi2020, Balbi2015 and BalbiNov2011TMdMl each register a "moisture"
property beside their fuel properties, which is what the property-shift
fix addresses. Supply a temperature and a moisture layer from the probe
so all three can run, and add them to the sweep. Reverting the "moist."
prefix narrowing turns Balbi2020's monotonicity red, so the fix is now
covered.
Run duration is calibrated per model rather than fixed. Spread rates
differ by more than two orders of magnitude across these six models, and
one duration either drives the fast ones into the domain edge, clipping
the dry end of the sweep, or leaves the slow ones so close to the initial
front that perimeterResolution quantises the displacement and adjacent
moistures collide. The calibration iterates, because a Balbi front
accelerates as it develops and a rate measured over a short trial
underestimates the eventual one. All six models now land between 350 and
715 m in a 2000 m domain, well clear of both failure modes, and the
sweep keeps resolving if any model is recalibrated later.
Runs in about 40 seconds.
* ci: run the model invariant suite on Linux
Adds invariants.yml, which builds the pyforefire extension and runs
tests/python/test_moisture_invariants.py on pushes to master, pull
requests against it, and manual dispatch.
A separate workflow rather than a job in main.yml, because the suite
needs the Python extension and install-forefire.sh does not build it:
outside a wheel build CMakeLists defaults FOREFIRE_BUILD_PYTHON to OFF,
so main.yml produces only the native interpreter. The job installs the
package into a venv instead of system Python, which keeps it clear of
PEP 668, and pip's build isolation supplies scikit-build-core and
pybind11 from pyproject.toml.
Checkout skips LFS, unlike main.yml: the suite builds its fuel, wind,
temperature and moisture layers in memory and needs no fixtures. Build
and suite together take about a minute locally.
This gives the repository a check that can distinguish a physics fix
from a physics regression. The runff test compares ForeFire against
frozen ForeFire output, so a deliberate model change breaks it while an
accidental one that leaves real_case.ff untouched passes.
Also documents the suite in TESTING.md, including the requirement that
each probe run in its own interpreter, and corrects the claim there that
nothing under tests/python is validated in CI.
* ci: run the invariants workflow on pull requests to dev
Matches the change made to main.yml and macos.yml: the workflow only
triggered on master, so nothing integrating into dev would run it.
* mv old files
* done
* Merge branch 'master' into dev
* test(http): pin the HTTP command server's behaviour before hardening it
Nothing tested the HTTP server, and it is due for changes to request
parsing and path resolution. These seven cases record what it does today,
so a regression shows up here rather than as a blank map in a browser.
Two behaviours the web console depends on and that any hardening must
preserve:
- POST / with a body of "ff:<command>" reaches the interpreter. That is
the entire UI-to-engine channel (forefireGUI.js sendCommand).
- Files resolve against the working directory. The console issues
plot[filename=fuel.png], ForeFire writes fuel.png beside the case,
and the browser fetches /fuel.png as a static file. A document root
that does not cover both htdocs and the working directory would break
every overlay.
Deliberately not covered: path traversal. Asserting the current
behaviour would document an unfixed weakness in a public repository. Its
test belongs with the fix, when the assertion can be that the request is
refused.
Two problems found while writing this, neither fixed here:
- HttpCommandServer.hpp was not self-contained. It used std::ifstream
without including <fstream> and an unqualified cout, so it compiled
only when included after a header that supplied both. Fixed, because
the test cannot include it otherwise; no behaviour change.
- stop() deadlocks. It closes the listening socket and then joins the
accept thread, but closing a socket does not reliably wake a thread
blocked in accept() on Linux, so the join never returns. The suite
therefore starts one server and never stops it. Fixing that needs
shutdown() or a self-pipe and belongs with the hardening work.
* test: add a concurrency stress test for free-threaded builds
Eight simulations in threads, each required to reproduce the fire node
count it produces when run alone, plus a case hammering object
construction to exercise the shared id counter.
Skips when the GIL is on, since every call into the extension is
serialised there and no race can show. Under PYTHON_GIL=0 on a
free-threaded interpreter it segfaults or returns wrong counts, which is
the point: it gives the shared-state work in #175 a failing test to fix.
Deliberately not wired into CI. On an ordinary interpreter it would skip
and report green while proving nothing; it earns a job once it can pass
for the right reason.
Refs #175
* fix(core): make the atom id counter atomic and the parameters singleton safe
Two races that the GIL currently hides.
ForeFireAtom::instanceNRCount was a plain long incremented with ++ to give
every atom its id. Concurrent construction could hand the same id to two
objects. It is now std::atomic<long> with a relaxed fetch_add, which is all
the ordering distinct ids need.
SimulationParameters::GetInstance used the textbook unsafe lazy singleton,
so two threads could both see null and both construct, then diverge with
different parameter objects. It is now a function-local static, whose
initialisation C++11 guarantees runs once. The unused private member is
gone.
* refactor(core): give each string representation its own buffer
StringRepresentation kept the output buffer, the nesting level and the
GeoJSON ring aggregation as statics and file globals, so two
representations in one process built into the same state. All of it is
per-instance now.
The file-scope 'outPattern' and 'domain' globals were shadowed by
members of the same name in every member function, so they were dead;
removed with the rest.
writedOnce stays a file global on purpose. Making it per-instance is the
one part of this that is observable: output() skips a representation
whose domain has no front once anything has been written, so the second
representation a coupled session creates would gain a first output file
it does not produce today. That is a behaviour decision for the coupled
runs rather than a mechanical one, and it belongs with the state work.
outputstr stays public because Command::dumpString reads it directly.
rungeojson.ff produces byte-identical output to dev, both files
(md5 5598d4b548bd7826546815f822b01073 and 177683c7bb42b267be6baa1d98ef5acd).
Refs #175
* fix(tools): restore genForeFireCase.py, with the axis order fixed
The script that writes the NetCDF landscape file was deleted in ac0baba
while four documents kept describing it, so anyone following the docs to
build their first case hit a file that was not there.
Restored from ac0baba^ with three fixes, since it was not usable as it
stood:
- The 3-D and 4-D paths were broken. Dimensions were read off the array
as (NY, NX, NZ, NT) while the variable was created as (NT, NZ, NY, NX)
and assigned untransposed, so a 4-D field raised a broadcast error and
a 3-D one never had its NT dimension created. Fields are now indexed
outermost axis first, which is what prealCF2Case.py in the same
directory settled on when it fixed its own copy.
- scipy.io.netcdf is deprecated and scipy is not a project dependency.
Uses netCDF4, which the tests already require, writing the same
NETCDF3_CLASSIC format prealCF2Case.py writes.
- parametersProperties has seven required keys and the docs called them
optional. They are checked before the file is opened, and named in the
error, rather than raising a bare KeyError over a half-written file.
tests/python/test_genforefirecase.py builds a landscape, loads it in
ForeFire, ignites and steps, and pins the 3-D and 4-D shapes. The four
documents now describe what exists.
prealCF2Case.py still carries its own copy of both functions. Merging
them is a change to a working script and is left for its own commit.
Closes #167
* ci: add a blocking AddressSanitizer job
ForeFire reports zero ASan errors today on both test paths, so the job
can be blocking from the day it lands rather than after a cleanup. The
point of turning it on now is to keep that property while the ownership
work in #175 starts moving allocations around.
-DFOREFIRE_SANITIZE=<list> passes <list> to -fsanitize= on the compile
line, the executables and the shared library, and switches the
optimisation flags to -g -O1 -fno-omit-frame-pointer: the default
-O3 -flto -fomit-frame-pointer set makes the reports unreadable. Having
it as a build option rather than raw CMAKE_CXX_FLAGS in the workflow
means the same build is reproducible locally.
The job runs the unit suite and runff under ASAN_OPTIONS=detect_leaks=0.
Errors block; leaks do not, and that is deliberate. Nothing owns a
PropagationModel (#159), so ASan reports every one as leaked, and a job
that is permanently red is a job everyone ignores. The leak count still
runs as an informational step so the figure stays visible, and it can be
made blocking once #159 lands.
Verified in ubuntu:24.04, which is what ubuntu-latest runs:
build with -fsanitize=address clean
unit suite, detect_leaks=0 4/4, 0 errors
runff, detect_leaks=0 passes, KML and NetCDF match
leak report, detect_leaks=1 exits 8, ~4.7 MB reported
Negative control: reintroducing the double free fixed in #157 into
ForeFireV1HeatFluxModel turns the job red with
ERROR: AddressSanitizer: attempting double-free
SUMMARY: AddressSanitizer: double-free in operator delete[](void*)
so the job catches the class of bug this project has actually had.
Closes #162
* fix(tests): make runANN work, and run it in CI
runANN has failed on line 2 since it was committed. It diffs the model
predictions against result.txt.ref, and that file is not in the
repository:
diff: result.txt.ref: No such file or directory
ANN results differs from reference.
Renaming the committed result.txt would not have fixed it. Regenerating
that output changes a handful of lines in the last digit (12412.1
against 12412.2), so an exact diff is red on any machine but the one
that produced the file.
ANN_test already computes the root mean squared error between what the
network predicts and what the model produced, and run.bash was throwing
that line away with `grep result`. It is now what the test checks: no
reference file, no dependence on the last digit, and it fails when the
network stops approximating the model.
The tolerance is calibrated against this fixture:
trained network 0.0235
predicting the mean of every output 0.0966
normalisation weights scaled by 1% 17817
0.05 sits between the first two, so a network that ignores its inputs
fails rather than passing. Anything that genuinely breaks the arithmetic
lands orders of magnitude away.
The fixture is weak, and that is recorded rather than papered over: the
expected outputs take four distinct values spanning 1.1 in 12412, which
is why the margin between a working network and a constant one is so
narrow. Better inputs would make this a much stronger check.
Wired into main.yml. This is the only coverage for ANNPropagationModel
and BMapLoggerForANNTraining: both read a .ffann network in their
constructor and abort without one, so the unit suite cannot construct
them. ANN_test is already built there, since FOREFIRE_BUILD_TOOLS
defaults on outside wheel builds.
Also:
- result.txt is removed. It was the golden file for a comparison that
no longer exists, and ANN_test regenerates that output on demand
with its `print` argument.
- clean.bash deleted results.txt, which is not the name of anything
the suite writes. There is nothing to clean now.
- TESTING.md said runff was the only test validated in CI, which has
not been true since the unit suite and the moisture invariants
landed.
Verified in ubuntu:24.04: same RMSE as the host to seven digits, which
is the portability this replaces the exact diff for.
Closes #163
* docs: add a CHANGELOG
The repository has seven tags and six GitHub releases, and no file that
tells a user what changed between them. `pip install forefire` makes that
worse: someone pinning a version has the release notes on GitHub and
nothing in the tree.
CHANGELOG.md reconstructs every release from the release notes and the
commit history. Claims that could be checked were checked against the
tags: listenHTTP and RothermelAndrews2018 are absent at v1.2 and present
at v2.0, the geojson dump mode is absent at v1.1.10.
[Unreleased] lists only what is on this branch, #154 and #155. The dev
branch carries a longer list in its own copy, which supersedes this one
when dev is merged.
CONTRIBUTING.md now asks for an entry, so the file stays current instead
of being reconstructed again in a year. Its link to a testing page
pointed at a Read the Docs URL that has never existed; it points at
TESTING.md, which nothing in the repository linked to.
(cherry picked from commit cc7e1789c66432aec56673993c3b6e198e0525a0)
* docs: record the dev-only work, and the options it added
The CHANGELOG landed on master carries only #154 and #155 under
[Unreleased], because that is all master has. This adds the ten pull
requests merged into dev since: the unit suite, the moisture invariants,
the sanitizer job, the HTTP characterisation tests, the threading
reproduction, and the shared-state and double-free fixes.
Two things went stale on dev while that work landed, both of them ours.
The README's CMake option table stopped at six options; #156 and #180
added FOREFIRE_BUILD_TESTS, FOREFIRE_ENABLE_WARNINGS,
FOREFIRE_WARNINGS_AS_ERRORS and FOREFIRE_SANITIZE. And CONTRIBUTING told
contributors to run `cd tests && bash run.bash`, which does not reach the
C++ unit tests at all; ctest does.
* docs(rtd): document the pip install path
The documentation site had no mention of pip, pypi or wheels -- `grep -rni
'pip\|pypi\|wheel' docs/source` returned nothing -- while the README's
headline instruction is `pip install forefire`. Every reader arriving at the
site was sent to build from source.
- installation.rst leads with pip: platforms, what the wheel contains, and
the three options wheels turn off (MPI, -march=native, ANN_test), then the
source build as before.
- The NetCDF prerequisite said 'Verify which one is actually required by the
current CMake setup', leaving the reader to answer the documentation's own
question. CMakeLists.txt looks for netcdf_c++4 / netcdf-cxx4 / netcdf-cxx
and its failure message already lists the package per distribution; that
table is now in the page, along with the note that libnetcdf-cxx-legacy-dev
is a different API.
- The FOREFIRE_* build options are documented, with their real defaults.
- quickstart.rst offered Docker only, calling it 'the quickest way'. A pip
route comes first now; the Docker walkthrough is unchanged.
- conf.py hard-coded release = '2.0.0', so the site advertised 2.0.0 while
the code was v2.5.0. It parses src/include/Version.h, as CMake and
scikit-build-core do.
Sphinx builds clean: the only warnings are the eleven doxygenclass lookups
that need the Doxygen XML RTD generates in its pre_build step.
* docs(tests): correct the test READMEs against the tree
Nearly every specific claim in tests/README.md was wrong:
- `idealized_wind.py` and `rothermel.fann` do not exist; the files are
`idealizedwind.py` and `Rothermel.ffann`.
- runANN was listed as needing `tensorflow` (or `torch`). It needs neither.
It runs `bin/ANN_test`, built from tools/runANN/ANNTest.cpp, and ForeFire
reads the .ffann network itself.
- runANN was described as comparing against reference outputs. It diffs
against result.txt.ref, which is not in the repository, so the suite fails
on its second line every time -- now stated, with a pointer to #163.
- percolation.py runs four fires, not three: one per entry in k_coeffs.
- idealizedwind.py writes no NetCDF, only 360wind.png, and by way of
ForeFire's plot[] command rather than matplotlib.
- runff has two entry points that do different things. run.bash runs three
scenarios and checks the artefacts exist; ff-run.bash runs two and compares
KML and NetCDF against references. CI calls the second, so only the second
can catch physics drift. The old text described neither accurately.
tests/python/README.md documented only farsite_flat.py, the one script that
cannot run as checked out, and did not say so. It now covers all four files,
keeps the download URL for flatland.lcp, and marks test_wheel.py as belonging
to cibuildwheel rather than to this suite.
* docs(commands): document emit, and de-duplicate clear
`emit` has been in Command::makeCmds since it was added, but in neither
app/forefire/commands.md nor the command reference. That file is not prose:
AdvancedLineEditor.cpp parses it into getCommandMan(), which drives Tab
completion, the help text and the syntax colouring. A command missing from it
is invisible to the console -- typing `emit` was rendered uncoloured, exactly
like a typo, while `save` beside it came out green.
commands.md also carried two '## clear' blocks. getCommandMan() assigns
cmdMan[key] as it walks the file, so the second silently replaced the first
and the longer entry was dead text. They are merged into one that matches what
Command::clear does: free the domain, cancel scheduled events, keep the
parameters.
Both files now cover all 22 registered commands, with no duplicates.
Verified: rebuilt and piped `emit`, `clear` and a nonsense word into the
console. The first two now colour green as recognised commands, the third does
not. Sphinx builds with no new warnings.
* docs: add what only exists on dev
The three cherry-picked commits describe master. This adds what this branch
has that master does not:
- The unit suite. tests/README.md said five sets of tests; with tests/unit
there are six, and it is a CTest suite rather than something run.bash
reaches, so the ctest command line is given.
- runANN works here (#183). The master text says the suite fails because it
diffs against a result.txt.ref that is not in the repository; that is
replaced by what it now does -- an RMSE tolerance of 0.05, sitting between
the trained network at 0.0235 and a mean-predictor at 0.0966.
- test_moisture_invariants.py and test_threading.py exist here. Both are
documented, including that test_threading.py does not pass and is not meant
to: it is the reproduction for #175 and needs a free-threaded CPython. None
of the three are pytest modules, so no pytest command is given.
- The four build options this branch added -- FOREFIRE_BUILD_TESTS,
FOREFIRE_ENABLE_WARNINGS, FOREFIRE_WARNINGS_AS_ERRORS and FOREFIRE_SANITIZE
-- join the table in installation.rst, and the wheel-defaults sentence
counts tests among what wheels turn off.
* docs(python): document the NumPy array path, no NetCDF or GIS
Add a User Guide page walking through a complete simulation driven entirely
from NumPy arrays via addIndexLayer/addScalarLayer, with no landscape .nc and
no GIS. The worked example was verified against a pip-installed ForeFire.
Link it from the index toctree and from the README's Python snippet.
Closes #168
* feat(python): add a forefire-validate landscape checker
Check a landscape .nc against a fuel table before a run: report any fuel index
present in the raster but missing from the table, flag a fuel/elevation shape
mismatch, and warn when elevation or wind is absent. This is the cheapest fix
for the most confusing failure mode, where a mismatch makes the simulation fail
or produce wrong results with no message.
The decision logic is kept pure and I/O-free (build_report), with a thin
netCDF4 adapter (read_landscape) that asks for netCDF4 only when a file must be
read, so the wheel gains no hard dependency. Exposed as the forefire-validate
console entry point.
test_validate.py loads the checker from source and covers the pure logic
without the compiled extension; the real-.nc case is skipped when netCDF4 is
absent. Wired into run.bash and documented in TESTING.md and the Python README.
Closes #169
* build: move stb_image_write.h into third_party/
1724 lines of public-domain code from nothings/stb sat in src/ among our
own, with nothing recording its version, its upstream, or its licence.
Patch it and the next update silently reverts the patch.
It now sits beside doctest, with all three recorded in the README, and a
note that Command.cpp is the only place that may define
STB_IMAGE_WRITE_IMPLEMENTATION.
third_party/ is now on forefireL's include path as well as the test
target's, in both cases as a SYSTEM include, so -Wall -Wextra stops at
code we maintain. PRIVATE on forefireL, since Command.cpp is the only
consumer.
Contributes to #161, which is where the file came to notice: 24 of its
428 warning lines were this one header.
* refactor(core): declare copy-assignment on FFPoint and FFVector
Both hand-write a copy-constructor and leave assignment to the compiler,
so each class carries two independently maintained definitions of how to
copy itself. They agree today. They stop agreeing the moment anyone adds
a member: the generated assignment picks it up, the hand-written
constructor does not, leaving `FFPoint b(a)` and `b = a` producing
different objects. FFPoint carries every coordinate in the simulation.
Writing the assignment operator out puts both next to each other, so a
new member has to be added to both or the omission is visible in one
file. Behaviour is unchanged: three doubles each, owning nothing.
Contributes to #161, which is where this came to notice: 65 of its 428
warning lines were the missing operator. The issue attributes all 65 to
FFPoint; 51 are, the other 14 are FFVector, which has the same shape.
* [python] helpers: import matplotlib only in the plotting functions
import pyforefire was loading matplotlib even when nothing is plotted.
* [tests] ignore PNG outputs in tests/runff
* fix(core): stop quit[] from ending the host process (#181)
quit is in the script command table, so it is reachable from the Python
binding and from the HTTP server, and it called exit(0). A host got no
exception and no traceback, its finally blocks and atexit handlers never
ran, buffered output was lost, and the status was 0 — so a batch job
recorded success having stopped early.
quit now releases the session and records the request; quitRequested()
exposes it and clearQuitRequest() forgets it. The shell in app/forefire
is what leaves: both the interactive loop and the HTTP listen loop check
it, so a served quit[] still stops the server. executeLoop stops reading
a script after one, rather than the process disappearing mid-file.
currentSession.params is no longer deleted. It is the SimulationParameters
singleton, owned by GetInstance() and shared with every other holder, so
deleting it left GetInstance() handing out a dangling pointer. That went
unnoticed only because exit() followed on the next line; without the
exit it would be a use-after-free on the next parameter read.
The safe topology error path called quit() too, so an internal failure
terminated the host on its own, with status 0. It reports and returns
error instead. The message moves to stderr and is no longer gated on
commandOutputs: a returned error nobody is obliged to check should not
also be silent.
tests/unit/test_quit_command.cpp covers it, and the CTest entries now
require doctest's closing 'Status: SUCCESS!' line. Without that a suite
whose process exits mid-run reports as a pass, because ctest sees 0 and
does not look for the summary that never printed — the first version of
this test passed happily with exit(0) still in place.
Verified with the reproduction from #160:
before quit[] / AFTER quit[] / FINALLY ran / ATEXIT ran, exit 0
against the previous output, which was 'before quit[]' and a dead
interpreter. Unit suite 5/5; runff KML and NetCDF match.
Closes #160
Co-authored-by: Antonio Leblanc <[email protected]>
* fix: clear[] crashed after a fire whose front had stopped (#194)
When the last event is taken out of the TimeTable, getUpcomingEvent left
head pointing at it. If that event then had an infinite time (its node
stopped), insert() deleted it and head dangled, so clear() deleted it a
second time. dropAtomEvents could likewise delete the event being processed.
Co-authored-by: Antonio Leblanc <[email protected]>
Co-authored-by: Jean-Baptiste Filippi <[email protected]>
* fix(core): give the propagation models and layers an owner (#182)
Building and destroying a FireDomain with a propagation layer cost about
183 kB every time, linearly and without limit. Two things were missing an
owner, and a third made ownership impossible.
The propagative layer was never freed: the delete in ~FireDomain was
commented out and the pointer nulled, which also removed any chance of a
later cleanup finding it.
The models were never freed either. propModelsTable and fluxModelsTable
are static, shared by every domain in the process, and nothing cleared
them. Each domain now records the entries it registered and releases
exactly those, before the data broker goes, since the models hold a
pointer to it.
The reason that could not simply be added: the FireDomain constructor
wiped both tables. Building a second domain therefore dropped the first
one's registrations, and its PropagativeLayer was left holding an index
that was empty until the new domain registered — at which point it
resolved to the *second* domain's model. Command.cpp builds exactly that
second domain for coupled runs. The wipe is gone; both tables are static
storage and already start out null.
Two smaller things on the same path:
- getFreePropModelIndex counted down through an unsigned index with no
lower bound, so a full table wrapped to SIZE_MAX and read far out of
bounds. getFreeFluxModelIndex bound-checked but then returned an
occupied index, silently overwriting a live model. Both now report
and return an out-of-range value that their callers refuse.
- DataBroker::addConstantLayer allocated an array, handed it to a layer
that copies it, and dropped it. Both delete[] lines were
sitting there commented out, one per branch.
Measured over 500 domains, each with a Rothermel layer: RSS growth falls
from 91,504 kB to 180 kB, and occupied model slots from perpetually
climbing to zero after each domain. Under ASan the unit suite goes from
4.7 MB leaked to none, so the leak check is now blocking for it; runff
still leaks ~200 kB on paths the suite does not reach and stays
informational.
tests/unit/test_domain_ownership.cpp covers the release, the
cross-domain clobbering and the loop. The model registry tests no longer
delete models by hand, which is a double free now that the domain owns
them; each takes its own sandbox instead, so destruction is exercised
the way it actually happens.
Closes #159
Co-authored-by: Antonio Leblanc <[email protected]>
Co-authored-by: Jean-Baptiste Filippi <[email protected]>
* fix: clear[] aborted after a fire front merged with itself and was too small (#197)
When a front merges with itself, FireFront::merge splits it into two loops.
If the loop it keeps has fewer than 5 nodes, it trashed that loop and the
front, then returned before the other loop was given a front. Those nodes
still pointed at the trashed front and kept moving. When two of them merged,
merge ran on the trashed front, found no node and trashed it again, so
~FireDomain deleted it twice.
The other loop is now trashed too. In every case seen it had 2 to 4 nodes,
below max_inner_front_nodes_filter, so the normal path would have trashed it
as well.
Co-authored-by: Jean-Baptiste Filippi <[email protected]>
* fix: segfault when a fire front was reduced to two nodes (#198)
When a front is down to two nodes, FireDomain::checkTopology trashes both
of them, but clears their front first, so eraseTopology could not move the
front's head off them. The front then kept a trashed node as its head: the
next burning scan counted its nodes from there, getNumFN failed, trashed the
front and returned 1, and storeVertices read the null head.
The head is now reset, as FireFront::dropFireNode does for a front of two
nodes.
Co-authored-by: Jean-Baptiste Filippi <[email protected]>
* build: remove two stray parentheses from the unit test list
#194 and #198 were merged on top of #181, each keeping its own closing
parenthesis, so CMake failed to configure tests/unit.
* fix(propagation): divide by the surface-to-volume ratio in Balbi2020 eq. C7 (#201)
/2*ls multiplied by ls instead of dividing, so Chi was close to zero.
With the radiant fraction right, a downslope with no wind gives a negative
rate of spread, so it is floored at zero.
Fixes #196
* feat(python): run a simulation in its own worker process (#200)
ForeFire keeps simulation state in process-wide statics, so two ForeFire()
objects in one Python process share it. ForeFireRunner gives each simulation
a separate process instead, as discussed in #175, and leaves the kernel and
ForeFire() unchanged.
The worker is a fresh interpreter (python -m pyforefire._worker) started
with fork+exec, since the caller may have threads. Calls are pickled over a
socket pair, numpy arrays included. Exceptions in the worker are raised in
the caller, and a dead worker raises WorkerDied instead of ending Python.
There is no fork of a running simulation yet.
* fix(python): raise instead of segfaulting before FireDomain[...] (#202)
* fix(python): raise instead of segfaulting before FireDomain[...]
Seven calls in the binding dereferenced the domain without checking it, so
ff["key"], addLayer, addScalarLayer, addIndexLayer or getDoubleArray before
FireDomain[...] crashed the Python process. They now go through
requireDomain(), which raises RuntimeError.
test_wheel.py checks each of them before creating a domain.
* docs(changelog): link #202
* fix(core): stop the data broker freeing the domain's propagative layer (#206)
Since #182 the domain deletes its propagative layer. loadData also
registers that layer with the broker, which deleted every registered
layer, so clear[] after loadData freed it twice. The broker now keeps it
listed for lookup and leaves deleting it to the domain, in its destructor
and when the layer's key is registered again.
reload_case.ff now ends with clear[], so the runff step of the
AddressSanitizer job covers this path.
Fixes #205
* docs(changelog): fill in [Unreleased] ahead of v2.6.0 (#207)
* docs(changelog): correct the v2.5.0 wheel platforms
PyPI has three wheels for 2.5.0 -- manylinux x86_64, manylinux aarch64 and
macosx_14_0_arm64. There is no Intel macOS wheel: the macos-13 runner was
retired and its job dropped in e43e0a0, which is an ancestor of the tag.
* docs(changelog): add the entries merged since v2.5.0 without one
#179, #181, #182, #189, #190, #192 and #197 had no entry, and the
listenHTTP entry did not credit #203. Version bumps, merges and the
documentation backports are left out.
---------
Co-authored-by: Jean-Baptiste Filippi <[email protected]>
* fix(core): addIndexLayer returns true when it registers a fuel table (#209)
The "table" branch registered the FuelDataLayer and then fell through to
the trailing `return false`. Nothing in the engine reads the result, only
the bindings do, so the layer worked while reporting it had not been made.
Co-authored-by: HugoFara <[email protected]>
* feat(build): FOREFIRE_ENABLE_NETCDF to build without NetCDF (#208)
* feat(build): FOREFIRE_ENABLE_NETCDF to build without NetCDF
`#include <netcdf>` in DataBroker.h pulled NetCDF into 48 of the 58 files
in src/. A new FF_NO_NETCDF guard compiles out every NetCDF-backed path
(loadFromNCFile, load/saveArrivalTimeNC, saveData, writeNetCDF, the PGD
form of createDomain, the NetCDF form of loadData). Each keeps a stub that
prints that the build has no NetCDF, so the command table is unchanged.
Default is ON: a normal build compiles exactly the same code as before
(tests/runff KML and NetCDF outputs match their references). Only adds
lines, no existing line changes. Needed for a WebAssembly build (#171),
and allows a slim pip wheel without NetCDF/HDF5.
* docs(changelog): FOREFIRE_ENABLE_NETCDF
---------
Co-authored-by: HugoFara <[email protected]>
* feat(build): compile the engine to WebAssembly (#210)
* feat(build): compile the engine to WebAssembly
Builds on FOREFIRE_ENABLE_NETCDF (#208) for #171. emcmake now configures
the WASM build with no arguments: EMSCRIPTEN turns off NetCDF, MPI, the
Python module, the unit tests and the termios-driven CLI shell, and turns
on an embind module that mirrors the pybind11 API with typed arrays in
place of NumPy arrays. The binding transposes C-order arrays into the
x-major order the data layers index with, as _pyforefire.cpp does.
The module links to a ~1 MB .wasm and runs: bindings/wasm/smoke.mjs
reproduces the NumPy documentation's array-only case and gets the same
front a native pyforefire does, 349 nodes.
* ci(wasm): build the module and run the smoke test under Node
* feat(wasm): run a fire in the browser on real terrain (#211)
* feat(wasm): run a fire in the browser on real terrain
A static page over the WebAssembly module (#171). landscape.mjs builds the
fuel and elevation layers from satellite and Terrarium tiles for any square
of ground, worker.mjs runs the whole simulation off the main thread and
reports why it ended, and terrain3d.mjs draws the result on a raw WebGL
heightfield that tilts up from the 2D map, with click-to-ignite by ray cast.
* ci(wasm): publish the browser demo to GitHub Pages on demand
* fix(wasm): return addLayer's result, and fail the smoke test on a missing layer
addLayer returned void, so JavaScript saw undefined and the demo's worker
treated the propagation layer as unregistered: every run failed with "a
layer failed to register". The engine side was fixed in #209; this is the
binding half, which #210 shipped without.
The smoke test printed each layer's result without checking it, which is
how this got through CI. It now exits non-zero if any layer is missing.
* feat(wasm): split the demo into Explore and Research modes
Explore is the default: click the map to light a fire, set the wind on a
dial or from a scenario, and watch it play back over a full-height map.
Research adds the propagation model, the two tuning factors, the numerics,
fuel moisture and bed depths, and the fuel class cut-offs, each labelled
with the ForeFire name it sets.
Each run now gets its own worker. The engine's state is process-wide
(#175), so a reused module kept the first run's propagation model and a
model switch did nothing.
* fix(wasm): seams, terrain spikes, and zooming out in 3D
The tile mosaic was drawn at fractional pixel offsets, so every tile edge
was antialiased: a dark seam in the imagery, and in the terrain tiles
half-blended pixels that decode to spikes hundreds of metres high. The
offsets are now rounded to whole pixels.
The wheel in 3D only moved the camera over the ground already loaded. It
now zooms the ground as in 2D, and the rebuilt mesh keeps the orbit pose.
setView dropped tilt.value, so the second terrain rebuild passed undefined
as the camera blend and drew nothing until the view was toggled.
* fix(wasm): keep the fire playing through a pan or zoom
Any pan or zoom cleared the run, because the fronts were drawn in the
current view's metres and would have landed on the wrong ground. Each run
now remembers the square it was computed on, its fronts are drawn wherever
that square sits in the view, and the ignition is measured against that
square rather than a view still waiting on its tiles.
* fix(wasm): leave unburnt islands unfilled, and keep the wind arrow off the terrain
Each front was filled on its own, so the inner front ForeFire keeps round
ground the fire has encircled was filled orange like the burn around it.
The fronts are now one shape filled even-odd, which leaves that ground
clear.
The wind arrow was painted on the 2D canvas, which is also the texture of
the 3D mesh, so it was draped over the terrain. It is now an overlay on
the map, hidden in 3D where map north turns with the camera.
* ci(wasm): publish the demo to GitHub Pages on every push to dev
* feat(wasm): colour the burn by arrival time and the front by rate of spread (#213)
The faded perimeter trail is replaced by two layers built from what the
engine already reports. Arrival paints the burnt ground by when it caught
fire, the arrival-time map ForeFire writes natively, with isochrones at a
round interval. Spread colours each segment of the front by its nodes'
rate of spread, which the worker now keeps from the `vel` in print[] instead
of discarding it.
Each frame's fronts become one Path2D in domain metres once per run, so the
arrival layer redraws through a transform as the view pans and zooms.
* ci: pull only the runff LFS data, cached between runs
* docker: skip the LFS check in the builder stage
* ci: build the WASM demo once and deploy it from the same workflow
* ci: skip the builds on docs-only changes
* ci: cancel superseded PR runs
* ci: move actions off Node 20
* docs: link the WASM demo from the README
* docs: give the WASM demo its own icon
* ci(docker): fix the image build on dev (#222)
* ci(docker): skip unit tests in the builder stage, tests/ is not copied
* ci(docker): copy third_party/ into the builder stage
* perf: inline hot-path accessors, make FFPoint and FFVector trivially copyable (#221)
* perf: define FireNode::getNext() and getPrev() in the header
Both are one-line accessors called through the PLT from the front-tracking
loops; defined in the header they are inlined.
* perf: define the one-line accessors of FFPoint, FFEvent, FireNode and FireDomain in the headers
Same change as the previous commit for the other accessors that show up in a
profile of the propagation loops.
* refactor: let the compiler generate FFPoint and FFVector copies (rule of zero)
f3190839 (#161) spelled out operator= so that the copy constructor and the
assignment cannot drift apart when a member is added. Removing both instead
gives that guarantee by construction: the compiler generates them from the
member list. The empty virtual destructors from the 2014 import go too:
nothing derives from either class.
Both become trivially copyable, 24 bytes instead of 32, and are copied and
passed by value without constructor and destructor calls.
* changelog: link #221
* docs: fix command examples that fail or are silently ignored (#223)
Every corrected example was run against a build of dev.
- Comments: a '#' after a command is read as a schedule operator, the
command does not run ("unknown post operator"). Moved trailing
comments to their own line and documented the rule.
- FireDomain: only sw, ne and t are read. BBoxWSEN and date= were
ignored, leaving the domain georeferenced at DefaultSWLngLat
(Corsica). Document setParameter[BBoxWSEN=...] before FireDomain.
- startFire: lonlat needs three values (lon,lat,z); (lon,lat) starts
no fire.
- trigger: the form is trigger[wind;vel=(...)]@t=..., not
fuelType=wind; t= inside the brackets is not read.
- plot: opt: is notation, not syntax; there is no area= key (BBoxWSEN,
BBoxLBRT, BBoxLBRT=active); size takes no parentheses; the arrival
time parameter is arrival_time.
The in-app help (app/forefire/commands.md) gets the same fixes.
* fix: use 3.59e-4 in the Rothermel wind exponent (#219)
* fix: use 3.59e-4 in the Rothermel wind exponent
10E-4 is 1e-3 in C++. Regenerates the runff references and the unit test pin.
* test: compare runff fronts by shape, not node by node
The front now has a different node count on each CPU (630 on the Linux runner, 636 on macOS, 637 elsewhere). Both comparisons run on failure, and the outputs are uploaded.
* test: set the runff front tolerance from the CI runners
* test: reword the tolerance comment
* build: -ffp-contract=off so every platform gives the same front; runff back to exact comparison
* changelog: the old Rothermel coefficient was 3.59e-3, not 1e-3; one list under Changed
* fix: stretch a fuel map that misses the domain, like the other layers (#220)
* fix: stretch a fuel map that misses the domain, like the other layers
When a NetCDF layer does not overlap the domain, DataBroker warns
"Failsafe Reprojection to whole domain extent" and builds the layer on the
domain extent. The fuel branch, added in c3a30da, computes that extent but
passes the file's own one to FuelDataLayer, so the map stays outside the
domain and every point reads its first cell.
* changelog: link #220
---------
Co-authored-by: Jean-Baptiste Filippi <[email protected]>
* chore(release): bump version to v2.6.0 (#214)
---------
Co-authored-by: HugoFara <[email protected]>
Co-authored-by: Hugo <[email protected]>
Co-authored-by: Yanis Srairi <[email protected]>
Co-authored-by: Yanis Srairi <[email protected]>
Co-authored-by: Jean-Baptiste Filippi <[email protected]>
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
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.
Bumps version to v2.6.0.
@filippi once this is merged, are you OK with merging
devintomasterand releasing the new Docker image and Python package? I can take care of that.