README: per-backend feature tier in the feature matrix#2468
Conversation
…2465) Replace the per-backend availability marks (✅️ native / 🔄 fallback / blank) in each feature table with per-backend feature-tier emojis from issue #2415: ✅ stable, 🔼 advanced, 🧪 experimental, 🔧 internal, 🚫 unsupported. A blank cell still means no implementation on that backend; cells that were CPU fallbacks become 🔼 advanced (documented execution mode, not native parity). Tier assignments are a first-pass classification derived from reading each module's implementation and test coverage. The conservative rubric from #2415 applies: stable requires a correctness baseline plus cross-backend parity evidence for that backend, the NumPy path can be stable while Dask/GPU paths remain advanced, and a green unit test on its own doesn't justify stable. Documentation only; the CRS support and datum sub-tables under Reproject are unchanged. Part of #2415. Closes #2465.
brendancol
left a comment
There was a problem hiding this comment.
PR Review: README per-backend feature tier
Documentation-only change; correctness, backend-dispatch, and test items are N/A.
Blockers
- (none)
Suggestions
- README.md:538 — Pycnophylactic. The interpolate/MCDA audit found the Dask path explicitly raises
NotImplementedError(iterative Laplacian smoothing needs global zone sums per step). By the tier definitions in #2415, an explicit-error combination is 🚫unsupported, not blank. Worth flipping the Dask and Dask+CuPy cells from blank to 🚫 so the table communicates the deliberate negative answer. - README.md:503-504, 511 — AHP Weights, Rank Weights, Sensitivity (Dask / CuPy / Dask+CuPy). Audit reports these as pure-numpy with no dask/cupy code paths. Marked 🔼 advanced on the assumption of silent CPU fallback. If a Dask or CuPy DataArray input actually errors, 🚫 unsupported is more honest. Quick check on a cupy-backed DataArray would confirm the failure mode before locking these in.
- README.md:280 — Min Observable Height. Row carries 🧪 and still has
*(experimental)*in the description. The textual marker is redundant once the tier emoji says the same thing; drop the suffix.
Nits
- README.md:144 — legend separator. Five-tier line uses
separators while the section TOC just below uses·. Using·in both would be a small consistency polish. - Stable claims on Dask/GPU columns. Several sections (multispectral, classification, morphology, focal core, hydrology core) ended up with
stablemarks across all four backends. #2415 is explicit that stable on Dask/GPU requires backend-specific parity evidence, not just a green unit test. A follow-up pass downgrading Dask/CuPy cells to 🔼 where per-backend parity isn't explicitly tested would be safer. Reasonable to land this as the first-pass best guess and tighten against the machine-readable registry from #2415 later.
What looks good
- CRS support sub-table and datum sub-table left untouched.
- No stray 🔄 anywhere in the feature tables.
- Legend explains both the fallback→advanced mapping and the blank-vs-🚫 distinction.
- Column structure, section anchors, and per-table headers all preserved.
Fixes from the PR review: - Pycnophylactic: Dask path explicitly raises NotImplementedError (dasymetric.py:502-503, 708-709), so the Dask and Dask+CuPy cells are 🚫 unsupported rather than blank. Blank reads as "not yet implemented"; 🚫 communicates the deliberate negative answer. - AHP Weights, Rank Weights: pure-numpy matrix/list utilities with no dask/cupy code paths. The Dask, CuPy, and Dask+CuPy cells are 🚫 unsupported rather than 🔼; calling them on a non-numpy backend is outside the function's contract, not a documented fallback. - Sensitivity: handles Dask input via .compute() (🔼 advanced kept). No CuPy path exists and the algorithm uses numpy.random / scipy.stats that would not work on cupy device arrays; CuPy and Dask+CuPy are now 🚫 unsupported. - Min Observable Height: dropped the trailing "(experimental)" from the description now that the 🧪 emoji conveys the same signal. - Legend separator: switched from to · so the tier legend matches the section TOC immediately below it. Deferred: the stable-on-Dask/GPU downgrade across multispectral / classification / morphology / focal core / hydrology core. That's a larger judgement pass and was explicitly framed as a best-guess first pass; better tightened against the machine-readable registry from #2415 when it lands.
brendancol
left a comment
There was a problem hiding this comment.
Follow-up review (after 771882e)
Three suggestions and one nit from the previous pass were applied; one nit was deferred with rationale.
Disposition of original findings
- Pycnophylactic Dask/Dask+CuPy → 🚫 — Fixed. Verified
dasymetric.py:502-503and:708-709raiseNotImplementedError; the docstring (:675) also says so. Now 🚫 on both cells. - AHP Weights, Rank Weights Dask/CuPy/Dask+CuPy → 🚫 — Fixed. Confirmed
weights.pyhas no dask/cupy code paths; these are pure-numpy matrix/list utilities. Marked 🚫 on all three non-numpy columns since the per-backend matrix's question doesn't really apply to them. - Sensitivity — Fixed. Confirmed
sensitivity.py:131,165handles Dask via.compute()(kept 🔼). No CuPy path and the algorithm usesnumpy.random/scipy.stats, so CuPy and Dask+CuPy are now 🚫. - Min Observable Height — drop
*(experimental)*— Fixed. The 🧪 tier emoji now carries that signal. - Legend separator nit — Fixed. Switched
to·so the tier legend matches the TOC just below it. - Stable-on-Dask/GPU downgrade across multispectral / classification / morphology / focal core / hydrology core — Deferred. This is a larger judgement pass touching ~60 rows; the user explicitly framed this PR as a best-guess first pass to pair with #2415's machine-readable registry. Better to tighten those marks against the registry when it lands than to make a second sweeping guess now.
Spot-checks on the new state
README.md:503-504AHP / Rank: ✅ 🚫 🚫 🚫README.md:511Sensitivity: ✅ 🔼 🚫 🚫README.md:538Pycnophylactic: ✅ 🚫 🔼 🚫README.md:280Min Observable Height: trailing(experimental)removedREADME.md:144Legend uses·separators consistent with the TOC
No new findings.
The first-pass classification was too generous with `stable`. The classification agents inferred stable from passing cross-backend parity tests, but GeoTIFF -- the precedent #2415 cites -- only marks NumPy/Dask local-file paths as stable and calls GPU experimental despite extensive tests. Stable requires a documented contract + release-gate testing, not green parity tests. Apply the same bar to the rest of the matrix: - NumPy `stable` only for canonical, literature-backed algorithms with golden-reference / hand-computed correctness tests. Custom / newer / utility / helper paths drop to `advanced` even on NumPy. - Dask `stable` essentially never outside GeoTIFF: chunk-boundary semantics are tested in places but not formally declared as a contract. Default to `advanced`. - CuPy / Dask+CuPy `stable` never: no per-backend tolerance contract. Default to `advanced` where parity tests exist, `experimental` for the few cells the audit found thinner (MCDA combine GPU paths). Net effect on the matrix: - `stable` cells: 533 -> 122 (NumPy core of canonical literature ops + GeoTIFF NumPy/Dask). - `advanced` becomes the default for working-but-uncontracted paths. - `experimental`, `unsupported`, and blank cells from the review follow-up are preserved.
brendancol
left a comment
There was a problem hiding this comment.
Follow-up: apply the GeoTIFF tier bar (171ee46)
Maintainer feedback on the first pass: too generous with stable. Agreed.
The classification agents were inferring stable from cross-backend parity tests passing. But the precedent #2415 cites is GeoTIFF, which marks only its NumPy/Dask local-file paths as stable and calls GPU experimental despite extensive tests. Stable requires a documented contract + release-gate testing, not green parity tests.
Stricter rules applied across the matrix:
- NumPy
stableonly for canonical, literature-backed algorithms with golden-reference / hand-computed tests. Custom / newer / utility / helper paths drop to advanced even on NumPy. - Dask
stableessentially never outside GeoTIFF — chunk-boundary semantics are tested but not formally declared as a contract. - CuPy / Dask+CuPy
stablenever — no per-backend tolerance contract anywhere outside GeoTIFF.
Net effect:
| tier | before | after |
|---|---|---|
| ✅ stable | ~533 | 122 |
| 🔼 advanced | rest | 502 |
| 🧪 experimental | 18 | 18 |
| 🚫 unsupported | 10 | 10 |
| blank | 16 | 16 |
stable is now concentrated where it should be: GeoTIFF NumPy/Dask, plus the NumPy paths of canonical literature ops (Aspect, Slope, Hillshade, Curvature, TPI/TRI/Landforms, multispectral indices, classification core, morphology core, focal core, hydrology D8 core, proximity core, fire, flood, MCDA combine core, IDW, A*, Reproject, etc.). Everything else is advanced — works, has tests, just lacks the GeoTIFF-grade contract.
Where the agents disagreed with that bar (e.g. they reported stable for Dask + GPU on multispectral and morphology because parity tests pass), I overrode and downgraded. The contract gate, not the test result, decides stable.
Closes #2465. Part of #2415.
The README feature matrix now shows the feature tier for each function on each backend instead of a bare availability mark.
Tiers are a first-pass, evidence-based classification drawn from each module's implementation and tests, using the conservative rubric in #2415: stable needs a correctness baseline plus cross-backend parity evidence for that backend; the NumPy path can be stable while Dask/GPU paths are still advanced; a passing unit test alone doesn't justify stable. The CRS support and datum sub-tables under Reproject are untouched.
When the machine-readable registry from #2415 lands, the table can be generated from it rather than maintained by hand.
Backend coverage
Documentation only — no code changes, no test changes.
Test plan