Conversation
…s layers Extends the #590 COG recipe to EVH, FBFM13, FVC, FVH, FVT, CBH, CBD, CC, CH. EVC and FBFM40 already have COGs and are skipped by the idempotence check. The COG step is independent of every open H3 decision on #515 (parent resolutions, EVH's reducer, FCCS), so it can run while those are settled. One thing worth stating explicitly, because this tranche is the first to mix the two kinds of layer: `-r near` / OVERVIEW_RESAMPLING=NEAREST is required here for BOTH, for different reasons. * the categorical ones are class codes, and averaging class codes is the silent corruption #590's recipe already guards against; * the continuous ones (CBH/CBD/CC/CH) carry `0 = Non-Forested` as a SENTINEL among real measurements -- 69.2% of pixels, measured 2026-09-07 -- so any interpolating resampler blends "no forest" with real canopy heights and invents a short canopy at every forest edge. So the comment now says never to switch these to bilinear/average "because they are continuous", which is exactly the reasoning that would otherwise look correct to the next reader. -9999 is Fill-NoData in every shipped CSV; 0 and the other codes stay real pixel values here and are excluded at the hex step, per the #590 pattern.
…s build set Generated with `cng-datasets raster-workflow`, one per layer, reducer and fill set per the measured decisions recorded on #515: fbfm13, fvc, fvh --hex-resampling mode --nodata "-9999" cbh, cbd, cc, ch --hex-resampling mean --nodata "-9999,0" The `0` in the canopy nodata list is the whole point and is not a typo: `0` is "Non-Forested" in every shipped canopy CSV, not a measurement, and it is 69.2% of valid pixels (measured 2026-09-07). Left in, `mean` would answer "average canopy height counting every prairie as zero" -- roughly a third of the honest value. --nodata takes a list, so this is the tool's own mechanism rather than anything bespoke. Native 10, parents 9,8,0 -- the set settled catalog-wide on #203 (parents are pure derivations of h10: h3_cell_to_parent(h10,8) reproduces the stored h8 with 0 mismatches over 520,962,530 rows, so h7/h6/h5 are derived at query time). One edit on top of the generated output: raster-workflow hardcodes completions:122, but CONUS occupies only 6 h0 cells at res 10. Running 122 would start 116 pods per layer that each localize a multi-GB COG and find no overlap. Replaced with the explicit H0S fan-out that AGENTS.md and #627 prescribe, as catalog/fire/k8s/mtbs/severity/mtbs-severity-conus-hex.yaml does. The reason is in each manifest so the deviation from generated output is not mistaken for drift. evh, evc and fvt are deliberately absent -- deferred on #515, each for a different reason (evh borderline pending a named consumer, evc needs fractions or a decode, fvt held back by choice though it now measures clean for mode). All nine COGs for this tranche are built and on S3; these read from them.
…why not The catalog runs ~90 generated raster recipes against ~26 hand-rolled, and the hand-rolled ones are disproportionately the large, expensive builds -- chelsa, landfire, mtbs/severity, nlcd, wrc-2, ghm, inhabit-v4. Hand-rolling starts exactly where the work gets hard, then propagates: the next agent copies the neighbouring recipe because that is what the catalog shows it. The failure mode is social rather than technical. A reviewer who cannot tell whether a deviation was deliberate tends not to challenge it, and someone running an agentic import may not feel placed to argue with the agent at all. So the fix is to make the deviation *stated* rather than to forbid it -- "the file says why" instead of "should I push back on this?". - scripts/check-generated-manifests.py: a recipe directory invoking cng-datasets must carry the generator banner, or declare `# HAND-ROLLED: <reason> (<issue>)`. Checks only the paths a PR touches, so the ~33 pre-existing undeclared recipes are grandfathered until edited. It ratchets; it does not demand a migration. - .github/workflows/check-generated-manifests.yml: runs it on catalog/** PRs. - AGENTS.md: the rule, both marker forms, and a table of the open cng-datasets gaps that legitimately force a hand edit today (#190, #191, #172, #39, #183) so a deviation cites a tracking issue and stays temporary. Also applies the rule to this tranche: - declares the three #590 landfire hex manifests, which fold h0 x layer x reducer into one indexed Job -- a shape raster-workflow cannot emit (#191, #172); - marks the CONUS fan-out edit on the seven generated manifests as `# DEVIATION from generated output (#191)`, so an edit on top of generated output is greppable and not mistaken for generator behaviour. Two gaps found while doing this and filed with tested MREs: datasets#190 -- --namespace is not stamped on generated step manifests, so applying one directly targets `default` and fails on RBAC in a way that reads like a cluster problem. datasets#191 -- hex completions fixed at 122 with no populated-h0 subset. CONUS needs 6, so 95% of pods pull a multi-GB COG to find no overlap; 812 wasted pod-starts for this tranche alone. Raster analogue of the closed #144. Those two are the most common entry points into hand-rolling a raster recipe; closing them removes the reason rather than the symptom.
…lly possible During the #515 canopy build, landfire-2024-cbd was published with 42.66% of its cells carrying the source fill code 32767, and a mean of 165.2 kg/m^3 for a variable whose valid maximum is 0.45. The --nodata list had omitted one of the two fill codes: 32767 is the source band's declared NoData, but gdalwarp wrote the COG with -dstnodata -9999, so 32767 survived as an ordinary pixel value. Every signal we normally trust passed it: - the k8s job succeeded on all indexes; - the partition was written, at a plausible size; - memory and runtime looked healthy; - the schema was right: int-typed, zero nulls; - verify-stac.py was clean, because the values are in-type and non-null. It was caught only by asking whether the numbers were physically possible. Worse than the flagged cells were ~1M where `mean` had BLENDED fill with real measurements -- those are not filterable afterwards and look like ordinary small values, so the script says so rather than implying a filter would do. The check is one MCP query per layer and catches a class of error that completion and schema signals structurally cannot. Two bugs of my own, both caught by testing against data already known good: - the first draft POSTed plain JSON to a guessed endpoint. The real MCP speaks JSON-RPC over SSE with a session handshake, so it now reuses verify-stac.py's MCPClient instead of carrying a second, wrong transport. - the MCP returns every column as a STRING, so '0' is truthy and the first working version reported FAIL on a clean layer with every count at zero. Verified in both directions on the completed landfire-2024-cbd: PASS --min 1 --max 45 --forbid 32767 --forbid 0 -> exit 0 FAIL --max 10 -> 69,599,754 cells out of range, exit 1 FAIL --forbid 45 (a real value) -> 20,451 cells flagged, exit 1 Also folds in the corrected hex settings for the remaining six layers: workers 8 / cpu 4 / 64Gi. Worker count turned out NOT to drive peak RAM in the 2-8 range -- cbd peaked ~37 GiB at 8 workers while cbh OOMed a 32 GiB pod at 2 -- so peak is dominated by a fixed per-h0 term and 8 is the throughput sweet spot rather than a memory risk. 64Gi sits above the measured peak with headroom while still scheduling far more easily than the 192Gi the generator implies.
A min/max range is close to vacuous for LANDFIRE class codes, because they are not contiguous. Measured from the staged legends: FBFM13 18 classes in 3 runs, 1..99 FVC 74 classes in 13 runs, 11..172 FVH 55 classes in 28 runs, 11..651 So `--min 1 --max 99` on FBFM13 admits 50, which is not a class at all, and my first pass at these bounds had FVC and FVH as 1..199 -- wrong on both ends, and off by 452 on FVH's maximum. `--allowed-csv URL` now asserts membership against the shipped legend's VALUE column instead. A `mode` reducer can only emit real class codes unless something has gone wrong -- fill leaking, or a blended-cell artifact -- which is precisely what this catches and a range does not. --min/--max become optional, for the continuous layers. Verified both directions against the already-published landfire-2024-vcc: correct legend (LF2024_VCC.csv) -> 520,962,530 cells, 0 outside, exit 0 wrong product (LF2024_FBFM13) -> 173,522,167 cells flagged, exit 1 The positive case doubles as independent confirmation that #590's vcc build is clean.
8 tasks
…rom the intermediate On #515, five fvc hex pods died with exit 137 and reason ContainerStatusUnknown/Error. I read that as OOM and raised the memory request 64 -> 128Gi. It changed nothing, because the pods were dying at 21-23 GiB of a 128 GiB limit. `kubectl describe pod` had the answer the whole time: Reason: Evicted Message: Pod ephemeral local storage usage exceeds the total limit of containers 40Gi. Measured inside a live pod: 34G /tmp/cng_collapsed_<id>.tif <- cng-datasets' fill-collapse intermediate 4.1G /tmp/cng-raster-cache <- the localized COG = 38G against a 40Gi limit The intermediate is ~8x the compressed COG and appears to be written uncompressed, so ephemeral needs sizing from the source, not left at a default. Filed upstream as boettiger-lab/datasets#209. AGENTS.md already states correctly that 50Gi is a default rather than a cap in geo-workflows (verified: the LimitRange sets `default: 50Gi` with no `max`), so this adds only the diagnostic distinction, which is what actually cost the time. Also raises fvh to ephemeral 80Gi -- fvc runs at 38 of 50, ~24% margin, which is tighter than it should be for the second-largest COG in the tranche -- and returns both to memory 96Gi, since memory was never the constraint.
…er; GB10 note
Three things worth having written down rather than re-derived.
1. 50Gi is a DEFAULT, not a cap, in BOTH namespaces. Verified by server-side admission
(`kubectl apply --dry-run=server`, which runs the real admission chain): 80Gi, 200Gi
and 1Ti are all admitted in geo-workflows AND biodiversity. Neither LimitRange carries
a `max`; no ResourceQuota in either mentions ephemeral. The ceiling is node free space.
2. The two eviction modes need different fixes, and they look identical in `kubectl get
pods` (exit 137):
- LIMIT eviction ("usage exceeds the total limit of containers N") -- the pod passed
its own limit. Unconditional, priority irrelevant, raise the limit.
- node DiskPressure eviction -- the node ran short, kubelet evicts in PRIORITY order.
So a large ephemeral request and `priorityClassName: opportunistic` compound: bigger
target, and first in line. The #515 run is at priority 0 after datasets#201, which is
what makes an 80Gi request tolerable.
3. Armada on 4x GB10 nodes (~4 TB local/ephemeral each, largely free) is coming, per the
repo owner. That inverts the current guidance: scratch-heavy raster work is constrained
by shared-node disk today, and largely will not be there. Recording it because it is
not derivable from the code or the cluster, and it changes when Armada is the right
answer -- from "escape the 200-completion cap" to "the natural home for scratch-heavy
and finely-chunked work" (datasets#173, #209).
…th the GRID, not the COG
I wrote that the fill-collapse intermediate is "~8x the compressed COG" and to size
ephemeral from the COG. That is wrong, and fvh disproved it: its COG is 3.74 GB against
fvc's 4.32 GB, and both produced an intermediate of exactly 34 GB.
The intermediate is uncompressed, so its size is grid pixels x bytes per pixel and is
independent of compression ratio. The LANDFIRE CONUS grid (156,336 x 101,538, Int16) is
31.7 GB by arithmetic, 34 GB observed with tiling overhead, on every layer alike.
Corrected rule:
ephemeral >= (grid pixels x bytes per pixel) + localized COG + margin
That is ~36-38 GB for this grid, which explains what actually happened: the canopy layers
cleared a 40Gi limit with ~4 GB to spare (not the comfortable margin I assumed), and fvc
evicted at 38.1 GB because its COG cache is 4.1 GB rather than ~2 GB. The difference
between the layers was never the intermediate -- it was the COG.
The old rule would have badly mis-sized any raster whose compression ratio differs from
these, in either direction.
make-stac-structure.py authors all seven and the public-landfire bucket collection. Two shapes in one tranche needing opposite treatment: CONTINUOUS (cbd, cbh, cc, ch) -- linearly scaled physical quantities. `mean` reducer, decode from the shipped CSV (KGM3 = VALUE/100, METERS = VALUE/10, CC_PERCENT = VALUE), and NO classification:classes: a continuous surface has no class list, and #628's rule is that anything listed there gets painted. CATEGORICAL (fbfm13, fvc, fvh) -- class codes. `mode` reducer, classification:classes from the shipped legend with fill codes REMOVED per #628. Every range, mean and `values` array is MEASURED from the published hex rather than taken from upstream documentation (#518). Verified: all 48 fvc / 40 fvh / 17 fbfm13 values present in the data are legend members, and no fill code appears in any class list. The four facts a consumer cannot recover from the data are all in the published text: - the decode factor -- without it a reader takes 12 for 12 kg/m^3 rather than 0.12; - `0 = Non-Forested` is a category, not a measurement, and is ~69% of valid pixels, so it is excluded and a mean over these layers is a mean over FOREST, not over all land; - cbh's top class (stored 100) means ">= 10 m" and holds ~19% of cells, so its mean is pulled toward that ceiling; - cc has no class below 10% cover -- sparse tree cover falls into 0 and is absent, so the layer cannot separate open woodland from treeless ground. One prose bug the gate could not catch: cc's decode sentence read "the stored value is not the physical quantity: the stored value is already a percentage to obtain percent", because the template assumed a scale factor and cc's is identity. verify-stac.py passed it -- it checks structure, not whether the sentence parses. Now branches on scale == 1.0. Bucket collection gets a SINGLE OWNER. make-stac.py's bucket_collection() knows only the original four layers, so running it against public-landfire/stac-collection.json would silently drop these seven; it is marked superseded in a docstring and this file carries the full nine-product roster. Two generators for one file is how the fractions claim drifted the first time (PR #655). Gates, all green: pre-publish verify-stac.py --no-data PASS x7 + bucket upload readback byte-identical x7 + bucket post-cluster verify-stac.py --bucket --dataset exit 0 x7 (includes the data-backed values == ingested DISTINCT check via the MCP) bucket collection 2 -> 9 children; public-landfire already registered in the root.
git rejects it outright -- "fatal: depth 0 is not a positive number" -- so the job failed at the fetch and the check never ran. It has been red since the workflow was added, which I missed because I tested the script locally and never looked at the CI run it was meant to gate. actions/checkout already uses fetch-depth: 0, so the base ref is present and no explicit depth is needed.
…uge NOT IN The categorical path pushed `NOT IN (<every legend class>)` to the engine. That is fine for an 18- or 74-class legend and does not complete at all for LANDFIRE FVT's 906 classes over 566 M rows -- I had to abandon the run and compare sets by hand. A checker that hangs is worse than one that fails: it looks like patience. And it hangs precisely on the layers most worth checking, since a large legend is where a stray code is easiest to miss by eye. Now: one DISTINCT aggregate (small, regardless of legend size, because the column is low-cardinality) and the membership comparison in Python. It also reports how many legend classes are unused, which is informative rather than a defect -- FVT uses 565 of 906, since a national legend covers vegetation types absent from CONUS. Verified three ways on published data: fvt + correct legend (906 classes) -> 565 present, 341 unused, exit 0 [previously hung] fvt + FBFM13's legend (wrong product) -> 563 strays named, exit 1 fbfm13 (17 present) / fvh (40 present) -> unchanged, exit 0
Moved into the build set on 2026-09-14. It was deferred as UNMEASURED, not as doubtful; measured on 2026-09-07 it came out cleaner than FBFM40, which was already published -- 262 classes in the sample yet a modal-share median of 0.80 and a minority winner in only 16.5% of cells, because vegetation TYPES occupy contiguous patches larger than a res-10 cell. That is the same finding that broke the "class count predicts mode adequacy" rule. Hex: mode, --nodata "-9999,32767", native 10, parents 9,8,0, 6 completions via --h0-subset. 80Gi ephemeral rather than the canopy layers' 40 -- its COG is 4.01 GB, the largest in the tranche, and the fill-collapse intermediate needs the whole grid on disk. Completed 6/6 with zero pod failures. Verified: 566,661,014 cells -- the same count as fbfm13, fvc and fvh, which is what the shared all-vegetated-land footprint should produce, against the canopy four's 267,118,010 forest-only count. Zero fill, zero nulls, and all 565 distinct values are legend members. STAC: classification:classes carries the FULL 906-class legend on the COG asset, while `values` on the hex carries the 565 classes actually measured in the ingest. The two differ by design and both are right -- classes describe what the raster can contain and drive the render colormap, `values` describes what the hex holds. 341 legend classes name vegetation types that do not occur in CONUS. At 292 KiB this is much the largest collection in the tranche; the full legend stays because `mode` can drop a class that is never any cell's plurality, so the hex's value set is a subset of the raster's and cannot stand in for it. Bucket collection 9 -> 10 children. Gates: pre-publish PASS, readback byte-identical, post-cluster verify-stac.py --bucket exit 0.
Member
Author
|
@cassiebuhler tagging you for review. The PR body covers what was built; this is what I'd most want a second pair of eyes on, roughly in order. 1. The STAC prose, more than the code. These descriptions are user-facing copy that the geo-agent quotes nearly verbatim to end users. They carry four facts a consumer cannot recover from the data, and if any is wrong or unclear the data is quietly misleading:
A prose bug already slipped past the gate once here: 2. The 3. The two new gates, which are the parts most likely to be wrong in ways nobody notices:
Both are tested in the failing direction as well as the passing one, which matters: my first version of the value checker reported FAIL on a clean layer (the MCP returns columns as strings, so Not in scope of this PR, both blocked on a decision rather than work: |
cassiebuhler
left a comment
cassiebuhler
left a comment
Collaborator
There was a problem hiding this comment.
Mostly good, but ran into these minor issues while reviewing.
- The ConfigMap and the standalone hex manifest disagree. Each
hex/<layer>/workflow.yamlrunskubectl apply -f /yamls/landfire-2024-<layer>-hex.yamlout ofconfigmap.yaml, and that embedded copy did not get the deviations applied to the sibling standalone file. Same on all eight layers:parallelism6 vs 3,backoffLimit: 0vsbackoffLimitPerIndex: 2+maxFailedIndexes: 6,opportunisticvs default priority, and noCNG_HEX_WORKERSat all vs8. The--h0-subsetand--nodatachanges were propagated, so the ConfigMap looks current. The missingCNG_HEX_WORKERS=8is the sharp part: the comment inlandfire-2024-cbd-hex.yamlrecords the tool default (48-64 workers) peaking at 190.5 GiB, and the ConfigMap copy requests 96Gi atopportunisticwithbackoffLimit: 0. Anyone following AGENTS.md Step 3 gets an OOM with no retry. Either sync the ConfigMaps or note in the directory that the standalone file is the one to apply. Worth knowing for the new gate: it accepts theGeneration command:banner anywhere in the directory, and here that banner lives inconfigmap.yaml, which is itself hand-edited, socheck-generated-manifests.pystructurally cannot catch this drift. make-stac-structure.py:77buildsPRESENT["fvt"]from/tmp/fvt-present-list.txtat import time, and nothing in the repo produces that file. Fresh clone crashes,--helpincluded. The other seven layers have their lists inline; fvt should too.fvtis in the PR (eight hex directories, plus thefeat(#515): add fvtcommit), but the PR body still lists it under "Deliberately not in this PR" and still quotes--nodata "-9999,0"rather than the shipped-9999,0,32767. "Not yet run" is stale too, sincecheck-hex-value-range.py, the AGENTS.md eviction measurements and the STACMEASUREDblock are all post-run.landfire-2024-fvt-hex.yamlis missing the# DEVIATION from generated outputblock the other seven carry, though it deviates the same way.- Stale comments, code is right in each case:
landfire-2024-structure-cog.yaml:37-42still says "every one of these four layers" and discusses EVC, which is not inPRODS, with the correct nine-layer block immediately below it; andlandfire-2024-hex-rest.yamlkeepsR = IDX/24, L = (IDX%24)/6above code that readsR=$(( IDX / 18 )), and keeps the "EVC gets half" rationale aboveWORKERS=(4 4 4)with EVC dropped. - Counts drift: "seven" collections in the
make-stac-structure.pydocstring and in the newSUPERSEDEDnote inmake-stac.py, against eight handled and nine COGs.
5 tasks
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.
Builds the seven-layer LANDFIRE 2024 structure/fuels build set decided on #515, plus WGS84 COGs for all nine remaining products in that tranche.
What decided the reducers
Every reducer here is measured, not inferred. Method and full tables are on #515; the short version:
fbfm13modefvhmodefvcmodecbhcbdccchmeanon the decoded valueThe canopy four turned out to be linearly scaled physical quantities with the decode in the shipped CSV (
VALUE ÷ 100= kg/m³,÷ 10= m,CC_PERCENT), someanapplies — confirming the suspicion recorded in #515's scope rather than the blanket "these are all coded bins, nevermean" rule that sat above it.The one line most likely to be "corrected" later
The
0is deliberate.0 = Non-Forestedin every canopy CSV — a category, not a measurement — and it is 69.2% of valid pixels, identical across all four (a shared forest mask). Left in,meananswers "average canopy height counting every prairie as zero", roughly a third of the honest value.--nodataaccepts a list, so this is the tool's own mechanism.Deviation from generated output — one, deliberate
raster-workflowhardcodescompletions: 122. CONUS occupies 6 h0 cells at res 10, so 122 would start 116 pods per layer that each localize a multi-GB COG and find no overlap. Replaced with the explicitH0S=(12 14 20 50 71 78)fan-out that AGENTS.md and #627 prescribe, matchingmtbs-severity-conus-hex.yaml. The rationale is inline in each manifest so it is not mistaken for drift.Everything else is generated output, untouched — including the per-layer
setup-bucketjobs, which are redundant for an existing bucket but harmless.COGs
landfire-2024-structure-cog.yamlextends the #590 recipe to the 9 products lacking a COG. All nine are built and on S3.raster-workflowdoes not generate a COG step — it takes an already-usable raster as--source-url— which is why this stays a separate job, consistent withpreprocess-cog.yamlin ca30x30 / carbon / connectivity.-r nearis required for both kinds of layer here, for different reasons, and the manifest says so: class codes must not be averaged, and the continuous four carry0 = Non-Forestedas a sentinel, so any interpolating resampler invents a short canopy at every forest edge. That second reason is the one a future reader would get wrong precisely because the layers are continuous.Deliberately not in this PR
evh-2024— borderline: aggregate survivesmode(1.74 pp) but the winner is a minority in 46% of cells. Deferred until a consumer is named, since per-cell and aggregate use want different reducers.evc-2024—moderejected outright (65% minority-winner, median share 0.32). Needsfractionsor the decoded surface; see Ingest: LANDFIRE 2024 CONUS — FBFM40 hex (finish 4 slices) + EVC decision #623.fvt-2024— deferred while unmeasured; it now measures clean formode(16.5%, 1.31 pp) and can join on request. Its COG is built either way.Not yet run
The hex jobs are committed but not applied — one hex workflow at a time per the namespace rule, and sizing should be measured on the first slice rather than inherited (#590 measured 115–144 Gi peaks; EVC OOMed at 192 Gi). STAC/README follow once the hex lands, so
verify-stac.pywill be RED at PR-open by design.