Uh oh!
There was an error while loading. Please reload this page.
Document the pip install path, and fix the stale test and command references - #186
Merged
antonio-leblanc merged 3 commits intoAug 13, 2026
Merged
Conversation
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.
antonio-leblanc
approved these changes
Aug 13, 2026
antonio-leblanc
left a comment
Collaborator
There was a problem hiding this comment.
ok only documentarion.
Commands.md is used in autocomplete on the forefire shell but ok nothin breaks
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 13, 2026
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 freeto 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.
Three findings from the documentation audit, the ones that send a reader somewhere wrong.
1. The documentation site never mentioned pip
Nothing. The README's headline instruction is
pip install forefire, and the site sent every reader to build from source instead — with a NetCDF dependency they do not need, since it is bundled in the wheel.installation.rstnow leads with pip: which platforms have wheels, what the wheel contains, and the three things it does not have (MPI,-march=native,ANN_test). The source build follows, unchanged in substance.quickstart.rstoffered Docker only, calling it "the quickest way"; a pip route comes first now, and the Docker walkthrough is untouched.Two things fixed along the way:
libnetcdf-cxx-legacy-dev. Verify which one is actually required by the current CMake setup."CMakeLists.txtsearches fornetcdf_c++4,netcdf-cxx4andnetcdf-cxx, and its failure message already lists the package per distribution. That table is now in the page, with the note thatlibnetcdf-cxx-legacy-devis a different, pre-C++4 API.conf.pyhard-codedrelease = '2.0.0', so the site advertised 2.0.0 while the code was v2.5.0. It parsessrc/include/Version.h, the same file CMake and scikit-build-core read.2.
tests/README.mdwas wrong in nearly every rowidealized_wind.pyidealizedwind.pyrothermel.fannRothermel.ffanntensorflow(ortorch)bin/ANN_test, and ForeFire reads the.ffannitselfresult.txt.ref, which is not in the repositoryk_coeffs360wind.png, via ForeFire'splot[]The runANN entry now says plainly that the suite fails, with a pointer to #163.
runffalso has two entry points that do different things, and the old text described neither:run.bashruns three scenarios and checks the artefacts exist;ff-run.bashruns two and compares KML and NetCDF against references. CI calls the second, so only the second can catch physics drift.tests/python/README.mddocumented onlyfarsite_flat.py— the one script that cannot run as checked out, becauseflatland.lcpis not in the repository — and did not mention that. It now covers all four files and keeps the download URL.3.
emitwas invisible to the consoletrans["emit"] = &emit;has been inCommand::makeCmdssince it was added, butemitappears in neitherapp/forefire/commands.mdnor the command reference.That file is not prose.
AdvancedLineEditor.cpp:32parses it intogetCommandMan(), which drives Tab completion, the help text and the syntax colouring. A command missing from it is invisible to the console's own help.Before, typing
emitrendered uncoloured — exactly like a typo — whilesavebeside it came out green. After:commands.mdalso carried two## clearblocks.getCommandMan()assignscmdMan[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 whatCommand::clearactually does: free the domain, cancel scheduled events, keep the parameters.Both files now cover all 22 registered commands, with no duplicates:
Not touching the open pull requests
This branch does not touch
CHANGELOG.md,CONTRIBUTING.mdorREADME.md— the only three files #184 and #185 change — so it merges in any order relative to them. No changelog entry is included for the same reason:CHANGELOG.mddoes not exist onmasteruntil #184 lands.Verification
doxygenclasslookups that need the Doxygen XML RTD generates in itspre_buildstep.This pull request, including its code changes and this description, was generated by Claude Opus 5, and reviewed manually before submitting.