fix: handle empty-feature depth slices in global rank - #372
Merged
Conversation
rank_soils_global builds a per-depth-slice feature matrix and passes it to gower_distances. When a depth slice has zero usable feature columns — e.g. the user recorded a depth interval but left texture/rock-fragment/color blank, which the bedrock column-filter can reduce to nothing — gower_distances fed a shape=(n, 0) array into SimpleImputer, raising "Found array with 0 feature(s)… minimum of 1 is required" and failing the whole ranking. Guard the gower call: for a 0-feature slice emit an all-NaN (n, n) distance matrix instead. The existing masked-average and NaN-infill steps already treat NaN distances as "no information", so the slice is ignored and components are ranked on the depths that do have data. An all-NaN matrix (rather than skipping the slice) keeps dis_mat_list positionally aligned with soil_matrix rows. Also make dis_max a single NaN-aware reduction over the stack so an all-NaN slice can't turn the max NaN via max()'s ordering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the per-slice gower computation into _slice_gower_distance so the empty-feature guard is unit-testable without a database or network, then add fast regression tests: - a 0-feature slice returns an all-NaN (n, n) matrix instead of crashing - the raw gower path still raises on a 0-feature array (characterization, so the guard's rationale is revisited if that ever changes) - a non-empty slice passes through to gower_distances unchanged Behavior of rank_soils_global is unchanged; this only names the guard and gives it a seam to test against. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
|
It would be fair to say that I reviewed lightly. Carissa or Garo might want to review as well if familiar with gower distance calculations. |
ssfrancis
approved these changes
Jun 26, 2026
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
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.
Problem
rank_soils_globalcrashes for some inputs with:(seen in production on tag
2026-06-24.1). The backend degrades gracefully (returnsALGORITHM_FAILURE), but the affected lookups get no ranking.Cause. Per depth slice,
rank_soils_globalbuilds a feature matrix and passes it togower_distances. When a slice has zero usable feature columns — the bedrock column-filter drops every property because the user'ssample_pedonrow is all-NaN at that depth —gower_distancesfeeds ashape=(n, 0)array intoSimpleImputer, which raises.Why now (regression). The crash line is old, but #368 (in
2026-06-24.1) changed user property arrays from a contiguous packed list to depth-indexed arrays that leaveNaNat any depth the user didn't record. That makes all-NaN slices a normal outcome for gapped / non-zero-start horizon data (exactly the pedons #368 calls out), turning a latent crash into a reachable one.Fix
_slice_gower_distance): a 0-feature slice returns an all-NaN(n, n)matrix instead of callinggower_distances. The existing masked-average and NaN-infill steps already treat NaN distances as "no information", so the slice is ignored and components rank on the depths that do have data. An all-NaN matrix (rather than skipping) keeps the per-slice list aligned withsoil_matrixrows.dis_max:max(map(np.nanmax, …))→np.nanmax(…)so an all-NaN slice can't make the max NaN viamax()'s ordering.Tests
New
soil_id/tests/test_global_rank_empty_slice.py(fast, no DB/network):(11, 11)matrix, no raisegower_distancespath still raises on 0 featuresgower_distancesunchangedAlso verified the live
test_global_integrationpath still ranks normally.Scope
This is a crash/robustness fix only — it stops the failure and lets ranking proceed on available depths. It does not make global rank use data below 200 cm; that depth space is still hardcoded to 200 (
min(bottom, 200), length-200 arrays) and is a separate, later change.🤖 Generated with Claude Code