Conversation
wpbonelli
added
requirement
5 tasks
wpbonelli
force-pushed
the
load
branch
2 times, most recently
from
March 7, 2026 22:10
133d3ba to
33ec812
Compare
Member
Author
|
superseded by recent and forthcoming work |
wpbonelli
added a commit
to wpbonelli/pyphoenix-project
that referenced
this pull request
Sep 12, 2026
PR modflowpy#284 ("input file loading") prototyped recursive namefile loading; develop has since shipped that mechanism for real (Simulation.load()/ Gwf.load(), modflowpy#348/modflowpy#349), superseding modflowpy#284's implementation. This starts fresh off develop rather than rebasing, salvages the two still-useful pieces (a docs reconciliation, a new corpus smoke test), and uses that test to find and fix everything wrong with the shipped loader. test/mf6/test_mf6_load_all_models.py parametrizes Simulation.load() over every model modflow_devtools.models exposes. It found the loader succeeding on ~9% of the real corpus -- and several "successes" had silently wrong griddata values, not just crashes. Fixed, in order: - basic.lark: blank/comment-only lines inside a block, missing trailing newline at EOF, empty files, comma as a token separator (surfacing and fixing an unrelated pre-existing regex bug: a misplaced "-" was silently widening word's character class into an unintended range) - structure.py: GRIDDATA blocks were parsed through the wrong code path first, silently corrupting every DIS/DISV field (a bare `TOP\n CONSTANT 0.` collapsed to the boolean True, broadcast as 1.0); INTERNAL arrays wrapped across multiple physical lines (or carrying per-line trailing annotations) were truncated to one line; OPEN/CLOSE-redirected griddata and list/period row data were unsupported; a variable-width cellid's element count was guessed from row length instead of read unambiguously from grid dims; TIMEARRAYSERIES/AUXILIARY-named period fields weren't recognized - reader: non-MF6 legacy content trailing the last real block (an mf5to15 conversion leftover) is now trimmed as a parse-failure fallback, never on the normal path; a block name repeating in one file now keeps the first occurrence instead of silently losing it to the second Every fix verified against real parsed values, not just non-crash, and checked for regressions via a full corpus probe plus the full test suite before moving to the next. Corpus pass rate: 9% -> 99% (239/242). The remaining 3 are confirmed fixture-quality issues, not flopy4 gaps (root-cause trail in load-corpus-gaps.md, untracked, same convention as plan.md): an LGR test case's external array file sized for the wrong sub-model's grid, and a third-party-exported .oc file omitting a field every one of the corpus's other 108 uses of it supplies correctly. Full suite: 643 passed throughout, same 7 pre-existing unrelated failures, zero regressions introduced at any step. Where this leaves the typed-grammar work (dfn2lark/TypedTransformer, mf6-object-model-plan.md Phase 3): untouched, still not on the load path -- develop's Simulation.load() remains basic-grammar plus this now much more tolerant structure.py reconstruction layer. Worth noting for whenever Phase 3 is picked up: most of what this branch fixed was real MF6 files being messier than a strict spec-driven grammar would accept (trailing inline annotations, multi-line-wrapped arrays, no final newline, duplicate blocks, comma-separated values, fields predating a DFN revision) -- a generated typed grammar would need equivalent leniency to preserve this pass rate, not just better structure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QM8E6Gyw13cFKJDt6WFaHx
wpbonelli
added a commit
that referenced
this pull request
Sep 12, 2026
#284 prototyped input file loading. The develop branch has superseded some of it. This PR salvages some docs and tests loading the examples and test models, and fixes some issues with the basic (untyped) loader. Future work will benchmark the basic loader and prototype/benchmark the typed loader to decide whether it's called for
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.
wiring it all up, super rough at the moment