docs(model-state): consolidate into one document, re-verified against icon4py main - #18
Closed
egparedes wants to merge 1 commit into
Closed
docs(model-state): consolidate into one document, re-verified against icon4py main#18egparedes wants to merge 1 commit into
egparedes wants to merge 1 commit into
Conversation
… main Distil the five-document model-state proposal set into a single coherent main document, and fold the `_specV2` review appendix into it (deleted). Structure now follows the working principles' vocabulary: requirements R1-R11 are split into four non-negotiable *constraints* (C1-C4) and seven *ranked goals* (G1-G7), the budgeted resource and user model are named, the honest stopping point (declare-and-check, no registry) is argued next to the full design, and M13 is restored. Every checkable claim was re-run against icon4py `origin/main` at 4c858a6a (2026-08-06), against `origin/physics_driver_tmx` for the TMX claims, and against icon4py's own venv for gt4py behaviour. Corrections: - gt4py named collections: `| None` in an annotation disqualifies nothing -- the subclasshook never inspects annotations. What disqualifies TracerState are the `= None` defaults. Newly found: a ClassVar also disqualifies a container (the `_field_type is _FIELD` clause), and PrognosticState became conformant when PR 1404 removed its `tracer` field. This changes what M11 has to check and when the failure surfaces. - E4: two production call sites, not three, and they do not differ in domain bounds; the defect is duplicated buffers plus one missing halo exchange. - E3: `qv` is zeroed on the IO path too, so the published temperature is a dry-air temperature and virtual_temperature is identically equal to it. - E6: 13 replication sites, not 7; 89 `.get()` calls at driver_utils:221-487. - E10: TMX declares 92 fields across seven containers; TmxInputState re-declares all six fields of the common DiagnosticState. - msimberg's spec v3 supersedes v2 and its shared mutable `Carry` is the run-time bucket the reachability test rejects -- the previously claimed agreement has inverted. - `tach check` enforces nothing (was listed as unverified): it resolves zero first-party imports and reports every module's deps as unused. - Metadata dict sizes and empty-units counts corrected; `units=""` for most geometry entries was wrong (11/51). The evidence appendix is now the single authoritative source for counts and file:line detail, re-verified throughout; the walkthrough is annotated where it has drifted. E7's three wrong-key bugs are confirmed still live and filable. status goes back to draft: the rewrite has not been reviewed by a human.
egparedes
added a commit
that referenced
this pull request
Aug 10, 2026
…on PR #18 Incorporate the useful material from the alternative consolidation (synthesize-model-state, PR #18) after independent re-verification against icon4py origin/main at de151fad8, so that PR can be closed: - E3 sharpened: the IO path is fully dry (qv zero too), so published virtual_temperature == temperature and the published temperature is dry-air (re-verified at driver_io.py:146-147). - E4 corrected: two production sites, identical domain bounds; the defect is duplicated buffers + one missing halo exchange. Also fixes this document's own stale line numbers (driver_states.py:290/:302 on main). - E6 corrected upward: ~12 replication sites (union definition), not 7; counts propagated to the walkthrough sketch and cost analysis. - E7: intended mapping independently confirmed at three sites; filable now. - E8 upgraded: the triplication is tested into place (driver_io reads + test docstring 'stated independently'). - E9 scoped: units='' holds for metrics/interpolation, not geometry. - E10 detailed: 92 fields across seven dataclasses, verified on the tmx branch 2026-08-05. - E1/E2: the fix's fallback branch restates the half-level extent -- E2's knowledge represented a third time, created by the fix. - C2 strengthened (no record descriptor; golden-file bindings test); C4 noted as satisfied by construction; G6 grounded in cached_dummy_field_factory. - gt4py ClassVar consequence: metadata as ClassVar is safe on Component, not on InputT; wiring-object-vs-program-argument must be declared and checked at seal(). - M3: muphys already declares both properties -- a first consumer waiting. - M6: icon4py has zero invalidation machinery (grep + no evict API). - A-vs-B table: E3/E4 need M5-lite either way -- the one admitted run-time mechanism; adoption order: file E7 bugs now, reuse the existing kind key, M13 lands with the winning composition layer. - msimberg v3: synthesized the two parallel readings -- composes on the declaration side, but D1/D2's shared mutable Carry + sampler cache is a run-time name-addressed store; bind-once at setup dissolves the conflict. - tach check enforces nothing today (namespace-package regression). - Open questions: restored the tracer-validation sign-off question; mass_flx_ic answered as science but still inexpressible as declaration. Not adopted from #18: its claim that PR 1301 is still open (it merged 2026-08-03, before #18's verification date); its three-appendix structure (this document stays single-file); its stale E6/PR-1360 figures.
Contributor
Author
|
Closed in favor or #19 |
egparedes
added a commit
that referenced
this pull request
Aug 10, 2026
…on PR #18 Incorporate the useful material from the alternative consolidation (synthesize-model-state, PR #18) after independent re-verification against icon4py origin/main at de151fad8, so that PR can be closed: - E3 sharpened: the IO path is fully dry (qv zero too), so published virtual_temperature == temperature and the published temperature is dry-air (re-verified at driver_io.py:146-147). - E4 corrected: two production sites, identical domain bounds; the defect is duplicated buffers + one missing halo exchange. Also fixes this document's own stale line numbers (driver_states.py:290/:302 on main). - E6 corrected upward: ~12 replication sites (union definition), not 7; counts propagated to the walkthrough sketch and cost analysis. - E7: intended mapping independently confirmed at three sites; filable now. - E8 upgraded: the triplication is tested into place (driver_io reads + test docstring 'stated independently'). - E9 scoped: units='' holds for metrics/interpolation, not geometry. - E10 detailed: 92 fields across seven dataclasses, verified on the tmx branch 2026-08-05. - E1/E2: the fix's fallback branch restates the half-level extent -- E2's knowledge represented a third time, created by the fix. - C2 strengthened (no record descriptor; golden-file bindings test); C4 noted as satisfied by construction; G6 grounded in cached_dummy_field_factory. - gt4py ClassVar consequence: metadata as ClassVar is safe on Component, not on InputT; wiring-object-vs-program-argument must be declared and checked at seal(). - M3: muphys already declares both properties -- a first consumer waiting. - M6: icon4py has zero invalidation machinery (grep + no evict API). - A-vs-B table: E3/E4 need M5-lite either way -- the one admitted run-time mechanism; adoption order: file E7 bugs now, reuse the existing kind key, M13 lands with the winning composition layer. - msimberg v3: synthesized the two parallel readings -- composes on the declaration side, but D1/D2's shared mutable Carry + sampler cache is a run-time name-addressed store; bind-once at setup dissolves the conflict. - tach check enforces nothing today (namespace-package regression). - Open questions: restored the tracer-validation sign-off question; mass_flx_ic answered as science but still inexpressible as declaration. Not adopted from #18: its claim that PR 1301 is still open (it merged 2026-08-03, before #18's verification date); its three-appendix structure (this document stays single-file); its stale E6/PR-1360 figures.
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.
Distils the five-document model-state proposal set into a single coherent main document, and folds the
_specV2review appendix into it (deleted).What changed
model-state.mdrewritten as one self-contained document. Requirements R1–R11 are split into four non-negotiable constraints (C1–C4) and seven ranked goals (G1–G7); the budgeted resource (reviewer attention) and the user model are named explicitly; the honest stopping point — declare and check, no registry — is argued on the page next to the full design; M13 (declared ordering constraints) is restored.model-state_specV2.mddeleted — it was a diff against the main document and stops making sense once applied.model-state_evidence.mdis now the single authoritative source for counts andfile:linedetail, re-verified throughout.model-state_walkthrough.mdannotated where it has drifted, with one claim superseded outright.Verification
Every checkable claim was re-run against icon4py
origin/mainat4c858a6a(2026-08-06 — four days newer than the previous pass), againstorigin/physics_driver_tmxfor the TMX claims, and against icon4py's own venv for gt4py behaviour. Most claims held. These did not:Field | Nonedisqualifies a gt4py named collectionx: int | Nonewith no default is conformant. What disqualifiesTracerStateare the= Nonedefaults. Changes what M11 must check, and when the failure surfaces (class definition → call time)ClassVaralso disqualifies a container (_field_type is _FIELD), and so does an empty dataclass.PrognosticStatebecame conformant when PR 1404 removed itstracerfieldvn → u,vcomputed 3× with divergent domain boundsqvis zeroed too, so the published temperature is a dry-air temperature andvirtual_temperatureis identically equal to it.get()calls atdriver_utils:221-487TmxInputStatere-declares all six fields of the commonDiagnosticState, all six tracers, andrho/wCarryis exactly the run-time bucket the reachability test rejects. The agreement has invertedtach checkenforces nothing (previously unverified)units=""for most geometry/interpolation/metrics entriesOne substantive disagreement with the thesis
"Every defect E1–E10 is decided once, before the time loop starts" is not quite right. E3 and E4 produce wrong values every step; only the wiring is fixed at setup. M5-lite is therefore a genuine run-time mechanism, and §4.5 now says so rather than folding it into the "no run-time object" claim.
Actionable independently of this design
E7's three wrong-key bugs are confirmed still live on
origin/mainand are filable today, with the intended mapping confirmed in three independent places:test_benchmark_solve_nonhydro.py:163—D2DEXDZ2_FAC1_MCassigned tod2dexdz2_fac2_mctest_benchmark_solve_nonhydro.py:98,test_benchmark_diffusion.py:105—EDGE_NORMAL_VERTEX_Vassigned todual_normal_vert_y, where the tangent was meantReview notes
statusgoes back todraftperAGENTS.md— the rewrite is AI-assisted and has not had a human review. That is a deliberate downgrade fromreviewed.setflagsobjection (cupy not installed — and v3 has relaxed the enforcement it targeted anyway), the PR review-load figures, and all prior-art line numbers outside ICON/LFRic/MAPL/gt4py.AGENTS.md.