fix: depth-align user horizon data in global rank (#368)#370
Merged
Conversation
knipec
approved these changes
Jun 24, 2026
rank_soils_global packed user-entered horizon values into a contiguous list (p_sandpct_intpl/p_claypct_intpl/p_cfg_intpl) with no depth alignment, then filtered that list by depth index via `index.isin(pedon_slice_index)`. When the user's horizons had a gap or did not start at depth 0, the packed list positions no longer matched the depth indices, so any data below a gap (or shifted by a non-zero start) was silently read as NaN and dropped from the similarity match. Build the property arrays indexed by true depth (0..199) instead, so list position == depth and the index filter aligns. This mirrors the approach already used in us_soil.rank_soils, which is why the US path was not affected. Verified on a live global lookup: with identical geometry but the deep post-gap horizon set to clay vs sand, the rankings now differ (previously identical because the deep horizon was dropped). Global snapshots will change for any test pedon with a gap or non-zero start; these are regenerated in a follow-up commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The depth-alignment fix in rank_soils_global changes the ranking for the two global test pedons whose user horizons are gapped or do not start at 0: - [37.33333,-5.4] (gap 25..125 cm) - [-2.06972,37.29] (first horizon starts at 37 cm) Previously these pedons silently dropped the post-gap / shifted horizon data; the snapshots now reflect the corrected rankings. The other 12 global pedons are contiguous-from-0 and are unchanged. Regenerated on top of the WRB description resync, so these diffs are rank/score only (no description text). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #368 depth-alignment fix removed its only callers in rank_soils_global (user property arrays are now built depth-indexed). Comment it out rather than delete, in case the fixed-length pad/truncate is wanted again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
johannesparty
force-pushed
the
fix/368-global-depth-interval-misalignment
branch
from
4af60ec to
96bb996
Compare
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
2 participants
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.
Fixes #368 — user-entered soil horizons were silently dropped/misaligned in the global ranking path when the horizons had a gap or did not start at depth 0.
Stacked on #369 (base =
chore/wrb-description-snapshots) — review/merge that first. This PR's own diff is just the two commits below.The bug
rank_soils_globalpacked user horizon values into a contiguous list (p_sandpct_intpletc.) and then filtered that list by depth index (index.isin(pedon_slice_index)). When the user's horizons had a gap or a non-zero start, list position no longer equalled depth, so data below the gap (or shifted by the non-zero start) was read as NaN and dropped from the similarity match.The fix (
9b5b914)Build the property arrays indexed by true depth so list position == depth and the index filter aligns. This mirrors
us_soil.rank_soils, which was already depth-indexed — so this is a global-only fix; the US path was never affected and needs no change here.Snapshots (
6f8942c)Two global test locations change because their input horizons exercise the bug:
[37.33333,-5.4]— gap from 25→125 cm[-2.06972,37.29]— first horizon starts at 37 cmThe diffs are rank/score changes (the previously-dropped horizon now contributes). The other 12 locations (contiguous-from-0) are unchanged.
Scope notes
us_soil.rank_soils) was already depth-aligned; no pending US work for this bug.docs/soil-depth-200cm-analysis.md, not part of this PR.🤖 Generated with Claude Code