Restore genForeFireCase.py as a general landscape writer - #179
Open
HugoFara wants to merge 1 commit into
Open
Conversation
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
HugoFara
force-pushed
the
fix/restore-genforefirecase
branch
from
August 12, 2026 19:17
be2af01 to
41af42e
Compare
HugoFara
changed the title
HugoFara
marked this pull request as draft
August 12, 2026 19:19
HugoFara
marked this pull request as ready for review
August 12, 2026 19:19
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.
Addresses #167. @filippi is the reviewer because he removed this script, and this proposes putting it back — that call is his.
The removal was intentional
I filed #167 framing the missing script as an oversight. The history says otherwise:
Consumer first, two months apart: the function was inlined where it was used, then the standalone file removed as dead code. This pull request undoes that.
prealCF2Case.py's copy has since diverged into the back half of the Meso-NH PGD → case pipeline, and needs a PGD file to do anything. So the repository has no general-purpose landscape writer, whilelandscape_file.rst,tools/README.md,READMEscripts.mdand the legacy user guide all describe one.The cheaper alternative is to delete the four references. That respects
ac0babamost directly and takes minutes. If that is the answer, say so and I will close this and send that patch instead.What is in here
Restored from
ac0baba^with three fixes, since it was not usable as it stood:(NY, NX, NZ, NT)while the variable was created as(NT, NZ, NY, NX)and assigned without transposing, which only succeeds whenNY == NTandNX == NZ. Fields are now indexed outermost axis first, matchingprealCF2Case.py.scipy.io.netcdfis deprecated and scipy is not a dependency. Now usesnetCDF4, writing the sameNETCDF3_CLASSICformat.parametersPropertieshas seven required keys thattools/README.mdcalled optional. Omitting one raised a bareKeyErrorover a half-written file; they are now checked before the file is opened.Verification
tests/python/test_genforefirecase.py(new), four cases: builds a landscape, hands it to ForeFire, ignites and steps; checks the 3-D and 4-D shapes using four different lengths, sinceNY == NTandNX == NZis what let the old broadcast succeed by accident; checks that a missing key is reported before anything is written.Unifying the two implementations is not attempted here — choosing between the combined
windvariable and separatewindU/windVis a question about what ForeFire should read.This pull request, including its code changes and this description, was generated by Claude Opus 5, and reviewed manually before submitting.
EDIT: rewrote for human readability.