Skip to content

Refuse unhonoured aggregations; add a real clear-day count to dft_stac_composite (#92) - #97

Merged
NewGraphEnvironment merged 10 commits into
mainfrom
92-dft-stac-composite-aggregation-count-si
Sep 30, 2026
Merged

NewGraphEnvironment merged 10 commits into
mainfrom
92-dft-stac-composite-aggregation-count-si

Conversation

@NewGraphEnvironment

Copy link
Copy Markdown
Owner

Summary

  • The bug. aggregation went straight into gdalcubes::cube_view(). That function reads any value it does not know as "none" and raises no error, so dft_stac_composite(aggregation = "count") returned plausible reflectance. The bug has been present since 0.18.0.
  • Refusing unhonoured values. dft_stac_fetch(), dft_stac_cube() and dft_stac_composite() now refuse any aggregation outside six values: min, max, mean, median, first and last. Each of the six was measured to survive a cube_view() round trip. The check runs before any network call. Case is ignored, and the value is hashed exactly as given, so no existing cache key moves; a round-3 review computed this against v0.19.2.
  • A real count. dft_stac_composite(aggregation = "count") now returns the number of distinct clear days per pixel.
    • It reads one day per time step (dt = "P1D", "first") and counts with reduce_time("count()").
    • It applies no scale, no offset and no offset split.
    • A pixel with no clear day is NA, never 0. gdalcubes returns 0 or NaN for such a pixel depending on chunk layout, and chunk layout follows parallel.
    • Counts are cached under their own family, count_<key>.tif, in INT2U with NEAREST overviews. So the 0.18.0-0.19.2 reflectance-as-count files are never read again.
  • Docs. They now say that the default Sentinel-2 mask includes snow, and where to find and delete the orphaned files.

Closes #92. The same silent fallback on resampling is filed as #96.

Measurement

Related Issues

  • Relates to NewGraphEnvironment/sred-2025-2026#16
  • floodplains#93 can now use aggregation = "count" as its issue body describes.

Test plan

  • devtools::test(): 1431 pass, 0 fail, 16 network skips
  • With DRIFT_TEST_NETWORK=true, the composite file passes all 115 tests, including the new live count test
  • devtools::document() and pkgdown::check_pkgdown() are clean
  • Every guard added here was mutation-tested: the defect was restored in a scratch copy and the test went red. That covered 20 mutations across aggregation passthrough, the offset split, zero->NA, the key family, datatype, overview resampling, the read step, file prefix, pixel_fn wiring, mask/months/query/resampling/cloud wiring, and case handling in each caller.
  • /code-check, 9 rounds over 3 diffs, each round ended by enumeration. The per-round findings are in planning/archive/2026-09-issue-92-aggregation-count/review-*.md.

Notes

  • The version is bumped to 0.20.0 because the count is new behaviour.
  • The cosmetic "offset split" message that stac_cube_items() prints for a count window straddling 2022-01-25 is left as is.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN

NewGraphEnvironment and others added 10 commits September 30, 2026 09:22
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
gdalcubes::cube_view() maps an unknown aggregation to "none" without an
error, so dft_stac_composite(aggregation = "count") returned reflectance.
dft_stac_cube(), dft_stac_fetch() and dft_stac_composite() now validate
against the six values measured to survive a cube_view() round trip
(min, max, mean, median, first, last), case-insensitively, before any
network call or cache lookup. count_values/count_images are held back:
they count items and every caller would scale them as reflectance.

Relates to #92

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
A count reads the window one day per time step (dt = "P1D", day
aggregation "first") and reduces with reduce_time("count(<asset>)"), so
overlapping MGRS tiles count one acquisition once and a masked tile does
not blank a clear one. No scale, no offset, no offset split (terra::cover
would drop the post side). A pixel with no clear day is NA: gdalcubes
returns 0 or NaN depending on chunk layout, which follows `parallel`.
The count keys under its own tag and writes count_<key>.tif (INT2U,
NEAREST overviews), so 0.19.x reflectance-as-count files are never read
and every composite key is unchanged. stac_cube_assemble() refuses an
unhonoured aggregation as a last guard.

Relates to #92

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
…#92)

Roxygen for dft_stac_composite() gains a section on counting clear days
(distinct days, snow in the default mask, NA for no clear day, a lower
bound where reads fail), the count_<key>.tif cache name, the valid
aggregation values, and a count example. dft_stac_cube() and
dft_stac_fetch() list their valid values.

aggregation_check() now returns the value as given rather than
lower-cased: every caller hashes it into a cache key, and lower-casing
moved the key of any mixed-case caller (v0.19.2 "Median" keyed
150c8ca5003bbe78), silently re-streaming cached cubes. Only the new
count family is normalised. A caller-level test drives fetch, cube and
composite with "Median" and records what reaches each key function.

Adds a DRIFT_TEST_NETWORK test of a live count and a COG-overview test.

Relates to #92

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
New chips-count stage of data-raw/benchmark_composite_bulk.R: the first
20 of #79's seed-79 chips (300 m buffers, 2023 Jul-Aug) as clear-day
counts, bands = "red". 577.5 s real, median 24.7 s a chip, 0.46 GiB
peak RSS (/usr/bin/time -l, committed as time_chips_count.txt); every
chip's highest count 5-9, no NA cells. The same 20 chips as median
true-colour composites took 1,051.6 s (#79).

Relates to #92

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
@NewGraphEnvironment
NewGraphEnvironment merged commit b10b028 into main Sep 30, 2026
1 check passed
@NewGraphEnvironment
NewGraphEnvironment deleted the 92-dft-stac-composite-aggregation-count-si branch September 30, 2026 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dft_stac_composite(aggregation = "count") silently returns reflectance, not a clear-observation count

1 participant