Skip to content

README: per-backend feature tier in the feature matrix#2468

Merged
brendancol merged 4 commits into
mainfrom
issue-2465
May 27, 2026
Merged

README: per-backend feature tier in the feature matrix#2468
brendancol merged 4 commits into
mainfrom
issue-2465

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

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.

  • ✅ stable · 🔼 advanced · 🧪 experimental · 🔧 internal · 🚫 unsupported
  • Blank still means no implementation on that backend.
  • Cells that used to be 🔄 (CPU fallback) are now 🔼 advanced — a fallback is a documented execution mode, not native parity.

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

  • CRS support sub-table and datum sub-table unchanged
  • No stray 🔄 in any feature-matrix row
  • Legend updated to describe the five tiers
  • Section anchors and column headers unchanged

…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.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 26, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 stable marks 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.
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-503 and :708-709 raise NotImplementedError; the docstring (:675) also says so. Now 🚫 on both cells.
  • AHP Weights, Rank Weights Dask/CuPy/Dask+CuPy → 🚫 — Fixed. Confirmed weights.py has 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,165 handles Dask via .compute() (kept 🔼). No CuPy path and the algorithm uses numpy.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-504 AHP / Rank: ✅ 🚫 🚫 🚫
  • README.md:511 Sensitivity: ✅ 🔼 🚫 🚫
  • README.md:538 Pycnophylactic: ✅ 🚫 🔼 🚫
  • README.md:280 Min Observable Height: trailing (experimental) removed
  • README.md:144 Legend 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.
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 stable only for canonical, literature-backed algorithms with golden-reference / hand-computed tests. Custom / newer / utility / helper paths drop to advanced even on NumPy.
  • Dask stable essentially never outside GeoTIFF — chunk-boundary semantics are tested but not formally declared as a contract.
  • CuPy / Dask+CuPy stable never — 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.

@brendancol brendancol merged commit eb68172 into main May 27, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

README feature matrix: show per-backend feature tier instead of bare availability

1 participant