Skip to content

genForeFireCase.py is documented in four places but is not in the repository #167

Description

@HugoFara

genForeFireCase.py builds the landscape file, without which ForeFire runs only on the shipped example. Users hit this about twenty minutes after the Docker demo works.

Documented in four places, present in none

git ls-files | grep genForeFireCase returns nothing, while these describe it:

  • docs/source/user_guide/landscape_file.rst:84
  • tools/README.md:12,20,62 — documents FiretoNC() argument by argument
  • tools/preprocessing/READMEscripts.md
  • docs/_legacy/UserGuide/ForeFireGeneralUsage.tex:170

It was removed in ac0baba ("clean more post-processing", Dec 2023).

It is recoverable and still works, with three fixes

I pulled it back with git show ac0baba^:tools/preprocessing/genForeFireCase.py and ran it. A 100×100 landscape loads, ignites and steps:

variables: ['fuel', 'altitude', 'windU', 'windV', 'domain', 'parameters']
LOADED OK
SIMULATED OK

But it is not a clean revert.

1. The 3-D and 4-D paths are broken. addFieldToNcFile reads the input shape as (NY, NX, NZ, NT):

ncfile.createDimension('%sNX'%fieldname, sp[1])
ncfile.createDimension('%sNY'%fieldname, sp[0])
ncfile.createDimension('%sNZ'%fieldname, sp[2])
ncfile.createDimension('%sNT'%fieldname, sp[3])
variable = ncfile.createVariable(fieldname, dvartype, ('NT','NZ','NY','NX'))
variable[:,:,:,:] = field

then creates the variable with the axes in the opposite order and assigns without transposing. That only succeeds when NY == NT and NX == NZ. A 4-D field fails:

ValueError: could not broadcast input array from shape (1,1,100,100) into shape (100,100,1,1)

Only the 2-D path is correct — and time-varying wind is exactly what a 4-D field is for.

2. scipy.io.netcdf is deprecated, emitting DeprecationWarning: … will be removed in SciPy 2.0.0. One-line fix to netcdf_file, or better netCDF4, which the project already depends on for testing.

3. parametersProperties has seven undocumented required keys. Omitting any of date, duration, refYear, refDay, year, month, day raises a bare KeyError after the file is partially written. tools/README.md calls this argument "the other optional properties you may want to put in the list" — it is not optional.

Suggested fix

Restore the file to tools/preprocessing/, fix the axis order, move off the deprecated import, document or default the required keys, add a smoke test (the sequence above is already one), then correct the four documents. Roughly half a day.

The alternative — deleting every mention — is much cheaper but leaves users with no way to build a case at all.


Drafted by Claude Opus 5 from a codebase audit. Reviewed by a maintainer before filing.

EDIT: rewrote for human readability.

Activity

  1. added a commit that references this issue on Aug 12, 2026
    41af42e
  2. HugoFara commented on Aug 12, 2026

    @HugoFara
    CollaboratorAuthor

    Correction to this issue: the removal was deliberate, and the wording above implies otherwise.

    fd4b1c8  2023-10-12  FiretoNC copied into prealCF2Case.py
    ac0baba  2023-12-18  genForeFireCase.py deleted ("clean more post-processing")
    

    Two months apart, consumer first. The function was inlined where it was used and the standalone file was then removed as dead code. That is ordinary de-duplication, not an oversight, and any restoration is a reversal of it rather than a repair.

    What has changed since is that prealCF2Case.py's copy diverged. It now sets the domain and parameters attributes by iterating the dicts with no required-key check, writes elevation as f4, and writes wind as a single combined wind variable in a potential-field form with the windU/windV calls commented out above it. It is the back half of the Meso-NH PGD pipeline and needs a PGD file to run.

    So the gap this issue describes is real — there is no general-purpose landscape writer, while four documents describe one — but the fix is a decision, not a correction. #179 proposes restoring the general writer and is left as a draft for that reason. Deleting the four references instead is cheaper and respects ac0baba more directly; it just leaves a new user with no way to build a case.

  3. antonio-leblanc commented on Aug 18, 2026

    @antonio-leblanc
    Collaborator

    ok merged to dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions