Refactor of the local data inference pipeline - #82
Draft
f-PLT wants to merge 51 commits into
Draft
Conversation
…. Remove persistant zoom
…new inference pipeline
Export FSC input cubes directly in the cell's native UTM CRS at scale=10 instead of round-tripping through EPSG:4326. The 4326 path left a grid- convergence tilt and a 0-valued border sliver around each tile, so adjacent cubes did not tile seamlessly. - eo_eval.py: new export_from_csv_utm_native — builds the request rectangle in the row's UTM crs, grows each cell by a buffer_m halo (neighbours overlap 2x buffer_m for a downstream mean-mosaic to reconcile), and exports at crs=<UTM>+scale=10 so EE resamples every band onto one shared 10m lattice. Verified: tiles are 108x108, origins on a common 10m grid, zero 0-border pixels, 80m overlap between neighbours, across all dates. - eo_eval.py: _export_for_polygon exports at scale=10 with the caller's crs (dropped an earlier crs_transform approach that failed with "inconsistent bounding boxes" on dates whose bands had mismatched native footprints). - build_aoi_cubes_gee_url.py: tile the AOI (Mode B), emit the GEE-UTM cube CSV, and drive the native export; --buffer-m controls the overlap halo. Assisted by Claude Code
url mode does a blocking getDownloadURL + download per cube, so it is network-bound and benefits from a small thread pool. Run the per-row exports through a ThreadPoolExecutor (default 4 workers); cloud/drive mode stays serial since it only fires batch submits that mutate the shared ee_task_list. - eo_eval.py: export_from_csv_utm_native gains max_workers (default 4); the per-row body is a closure dispatched via ThreadPoolExecutor for url mode. - build_aoi_cubes_gee_url.py: --max-workers flag wired through and logged. Workers are capped low deliberately: GEE throttles getDownloadURL, so a large pool returns 429s rather than speed. Assisted by Claude Code
…x duplication around grid management and reuse of existing cubes
# Conflicts: # src/snow_galileo/data/local_sources/grid.py # tests/test_local_sources/test_cube_csv.py
# Conflicts: # docs/local_data_processing.md # scripts/developer_scripts/bow_valley_inference_local/05_data_viewer.py # src/snow_galileo/data/local_sources/viewer/README.md
Commit 3a84403 removed grid.py's module-level DEFAULT_LEGACY_CSV and DEFAULT_AOI_PATH — correctly, since a library module should not hardcode a tests/fixtures path as a production default — but left four callers passing neither. This restores them as configuration rather than as constants. Mode A and mode B are two different ways to build a grid, and they take different inputs: mode A's cell list comes from a CSV that an AOI may optionally filter; mode B tiles an AOI and has no CSV at all. Forcing both through one signature is what made a required positional that half the callers could not supply. Split them: cells_from_csv(cube_cells_csv, aoi_path=None, require_fully_inside=False) cells_from_aoi(aoi_path, inset_m=0.0) build_cells/build_grid(mode, ...) # dispatch for config-driven callers Both paths now live on CubeSettings: cube_cells_csv (no default — a mode-A config names its own cell list) and aoi_path (defaults from LocalPaths, the same pattern archive_root and processing_root already use). LocalPaths keeps aoi_path; the Stage-1 clip stage and the viewer both read it and must not depend on Stage-2 cube-assembly config. Behaviour changes worth noting: - Mode A previously resolved its cells from tests/fixtures/sampled_cells_bow_ river_with_dates.csv, so the production path depended on a test fixture. cube.yaml now names configs/bow_valley/cube_cells.csv. That file was itself produced by filtering the fixture through the AOI, so re-filtering is idempotent: the grid is the same 344 cells (verified, dropped=0). - The AOI is optional in mode A. The 344-cell filter was an artifact of the Bow Valley parity work, not a contract; mode A over an arbitrary cells CSV is the training/validation-sampling case. - Three guards added, each for a silent failure rather than a loud one. A mismatched CSV/AOI pair keeps zero cells, and the sweep would otherwise run to completion over an empty grid and write nothing; require_fully_inside without an AOI would silently do no filtering at all. Deletes the placeholder-cube CLI in exporter.py. Nothing imports it, [project.scripts] is empty, and its only documented invocation (TASK-004 §6) still uses the pre-rename src.data.local_sources path, so it has been unreachable since the snow_galileo restructure. The placeholder export path it covered is exercised by seven tests. Follows the same removal as grid.py's CLI in 3a84403 and s1_snap.cli() in 8eb5242. Note that `make mypy` is no longer sufficient to validate these call sites: every dispatcher argument is optional, so a missing per-mode input type-checks and fails at run time. The new tests in test_grid.py cover each guard, and all three cube configs were verified by building their grids for real (A=344, B=21985, B=21985). CRS remains welded to EPSG:32611 — a cells CSV from another UTM zone is still rejected by load_cells. Generalizing that is TASK-017, added here. make mypy clean (was 4 errors). pytest: 365 passed (360 baseline + 5 new). Assisted by Claude Code.
Both Stage-2 inference entry points guarded against predicting on a cell with no
observations, and both read the loader's masks with the wrong polarity. The loader
emits 1 = masked (landsat_eval.py:630-631), the guard's comment claimed 1 = valid, so
`all(not mask.any() ...)` asked "is this cube perfect?" instead of "is it empty?".
It was worse than inverted: it was a constant False. `mask_prediction_era5` forces a 1
into `t_m` on every cube, so the generator short-circuited before reading anything else,
and `st_m` (location, derived from the cell centre) is never nodata, so flipping the
comparison alone would have left it equally unreachable. A 100%-nodata cell therefore
received a finite, plausible FSC derived from its coordinates alone, written to the COG
as a measurement and counted in `aoi_coverage_fraction`. No NaN, no exception, no log.
Existing outputs are unaffected: measured over all 7300 cubes on disk, zero would have
triggered the guard. The bug was latent.
The replacement, `_loader_bridge.has_no_spacetime_observation`, fires when all three
space-time masks are 1 everywhere. Static ancillary (DEM, WorldCover), non-spatial time
bands (coarse VIIRS, ERA5) and derived geometry (location) are excluded: none can support
a spatially-resolved FSC patch on its own, and including location would make the
condition unsatisfiable. It reads named `MaskedOutput` fields rather than positional
indices, because a positional lookup with a comment asserting the opposite polarity is
exactly what failed here.
One definition in `_loader_bridge.py`, shared by the driver and the script, replacing two
copies. The per-cell "cell_fully_masked" warning becomes a per-day count; at grid scale
the condition it reports would fire for every cell of a sparsely-covered AOI.
Tests: all three fakes in test_inference_driver.py encoded the inverted convention, one
of them labelling all-ones masks "all-valid". Left alone, five tests would have stayed
green while silently writing empty mosaics. They now build real `MaskedOutput`s, so a
double that disagrees with the loader raises instead of answering wrong. The placeholder
end-to-end test becomes the empty-cell regression test, and a new positive-path test
derives a real cube from tests/fixtures/gee_reference_patches, since a placeholder cube
can no longer reach the predict-and-write path.
AC-28 and TASK-015-PLAN.md are amended: the original wording ("all input groups masked")
was unsatisfiable for every possible input, and the plan deferred the polarity to "the
existing mask convention" rather than stating it.
Assisted by Claude Code.
The AC-28 guard fix cited `docs/agents/bugs/MASK_CHECK_BUG.md` from seven places in committed files. That path is not in the repository, so every one of them was a dead link for anyone but its author. The surrounding docstrings already say what the defect was and why the current shape avoids it, so the citations carried no information the reader could not get in place. Docstrings and comments only; no behaviour change. Assisted by Claude Code.
# Conflicts: # scripts/developer_scripts/fortress_mountain_basin/build_aoi_cubes_gee_url.py # src/snow_galileo/data/local_sources/grid.py
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.
Fixing shortcuts, cleaning up the code and generalizing utilities for use in other areas.
Base on recent changes introduced in #78
Current work in progress