Skip to content

Backport the documentation fixes to dev, with the dev-only entries - #187

Merged
antonio-leblanc merged 4 commits into
forefireAPI:devfrom
HugoFara:doc-pip-and-stale-refs-dev
Aug 13, 2026
Merged

Backport the documentation fixes to dev, with the dev-only entries#187
antonio-leblanc merged 4 commits into
forefireAPI:devfrom
HugoFara:doc-pip-and-stale-refs-dev

Conversation

@HugoFara

Copy link
Copy Markdown
Collaborator

Backport of the documentation audit fixes to dev, plus what only exists here.

The first three commits are cherry-picked from that branch unchanged. The fourth adds the rest.

What the fourth commit adds

  • The unit suite.tests/README.md said five sets of tests; with tests/unit there are six. It is a CTest suite rather than something run.bash reaches, so the ctest command line is given.
  • runANN works here (Make runANN work, and run it in CI #183). The master text says the suite fails, because it diffs against a result.txt.ref that was never committed. That is replaced by what it now does: an RMSE tolerance of 0.05, sitting between the trained network (about 0.024) and a predictor that ignores its inputs and returns the mean (0.097).
  • test_moisture_invariants.py and test_threading.py. Both documented, including that test_threading.py does not pass and is not meant to — it is the reproduction for Simulation state is process-wide, not per-simulation #175 and needs a free-threaded CPython with PYTHON_GIL=0. Neither is a pytest module, so no pytest command is given for either.
  • The four build options this branch addedFOREFIRE_BUILD_TESTS, FOREFIRE_ENABLE_WARNINGS, FOREFIRE_WARNINGS_AS_ERRORS and FOREFIRE_SANITIZE — join the options table in installation.rst, and the wheel-defaults sentence counts tests among what wheels turn off.

A note on the next dev → master sync

Unlike the CHANGELOG pair, these two branches are not superset-related: tests/README.md genuinely says different things on the two branches, because runANN passes on one and fails on the other. Ten lines differ that way, in three localised blocks (tests/README.md, tests/python/README.md, installation.rst).

If that conflicts on a future sync, take dev's copy — it describes the merged state correctly by construction.

Verification

  • The cherry-picks applied cleanly; no conflict resolution was involved.
  • ctest --test-dir build --output-on-failure, the command this adds to tests/README.md, was run: 4/4 pass.
  • tests/runANN/run.bash was run: 1424 inputs, RMSE 0.0236508 (tolerance 0.05). The figure in the README is stated as "about 0.024" rather than pinned, since it moves in the last digits between builds.
  • Sphinx builds with no new warnings.
  • No overlap with CHANGELOG.md, CONTRIBUTING.md or README.md, so this is independent of Backport the CHANGELOG to dev, with the dev-only entries #185.

This pull request, including its code changes and this description, was generated by Claude Opus 5, and reviewed manually before submitting.

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.
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 forefireAPI#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.
`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.
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 (forefireAPI#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 forefireAPI#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.

@antonio-leblancantonio-leblanc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs-only backport, verified locally against master's #184/#186 — no conflicts, cherry-picks apply cleanly.

@antonio-leblanc
antonio-leblanc merged commit b81f686 into forefireAPI:devAug 13, 2026
4 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@HugoFara@antonio-leblanc