mosaic: read tiles in place from S3, in a process pool; fix add_to_band - #60
Merged
Merged
Conversation
For mosaicking tiles that live on S3 (ATL1415 on MAAP DPS): - io_utils.glob_remote(pattern): the remote glob.glob, returning sorted URIs. - make_mosaic.py: --directory may be a URI, listed with glob_remote and read in place; --output must then be a local absolute path (h5py writes it). New --block_size (default for remote tiles: DEFAULT_REMOTE_BLOCK_SIZE) and -j/--workers. - grid.mosaic.from_list(block_size=None, workers=1): block_size reaches from_h5/from_nc for remote files -- without it s3fs's 50 MiB default reads most of each tile for a few fields. workers > 1 reads the files in a process pool (forkserver: fork after s3fs has started fails, "This class is not fork-safe"; threads give nothing, h5py's lock serializes the reads). Tiles are still added in list order, at most 2 x workers ahead, so the mosaic is identical to the serial one. workers=1 with no block_size is the original code path. Measured on 557 ATL1415 GL tiles on S3 (MAAP ADE): 40 km average 357 s serial -> 44.5 s with 8 workers; z0 (weighted) 802 s -> 195 s with 4 workers; both bit-identical to the serial mosaics. Each worker costs ~0.35 GiB resident. Fix: add_to_band sliced an in-memory mosaic with item[:,:,in_band], which returns a plain grid.data (grid.data.__copy__), then failed on update_spacing; it now converts back with mosaic().from_grid(), as the grid.data branch beside it already did. Hit by from_list(by_band=True) with any 3-D in-memory mosaic input. tests/test_mosaic_list.py: every read option against the original path (weighted by band / all bands / 2-D, replace, bands, all fields, unreadable tile, in-memory inputs), remote reads through fsspec's memory filesystem, block_size reaching open_remote, make_mosaic.py on a remote directory and its output check, and the add_to_band regression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI failed test_make_mosaic_remote_directory: the local run took its tiles in the directory's order (glob.glob), the remote run in sorted order, and a weighted mosaic depends on summation order in the last bit (reversing 9 test tiles: max |diff| 2.2e-16, same NaNs). The test passed on the ADE only because that directory happened to list in sorted order. Sorting makes a local mosaic the same on every filesystem and the same as the remote one. It can differ in the last bit from a mosaic made before, from an unsorted directory listing. test_make_mosaic_sorts_a_local_glob reproduces CI's condition anywhere (a glob that returns the files reversed); it fails without the sort. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
For mosaicking ATL1415 tiles that live on S3 (MAAP DPS jobs read them in place; nothing is copied to /home).
Changes
io_utils.glob_remote(pattern): the remoteglob.glob, returning sorted URIs.make_mosaic.py:--directorymay be a URI (listed withglob_remote, tiles read in place);--outputmust then be a local absolute path. New--block_size(default for remote tiles:DEFAULT_REMOTE_BLOCK_SIZE) and-j/--workers.grid.mosaic.from_list(block_size=None, workers=1):block_sizenow reachesfrom_h5/from_ncfor remote files. Without it, s3fs's 50 MiB default reads most of each tile for a few fields: 4 tiles pulled ~1 GB.workers > 1reads files in a process pool using forkserver. Forking after s3fs has started fails ("This class is not fork-safe"). Threads give no speed-up, because h5py's lock serializes the reads (measured).workers=1with noblock_sizeis the original code path.add_to_band:item[:,:,in_band]on an in-memory mosaic returns a plaingrid.data(viagrid.data.__copy__), which then failed onupdate_spacing. It now converts back withmosaic().from_grid(), as the neighbouringgrid.databranch already did. This affectedfrom_list(by_band=True)with any 3-D in-memory mosaic input.Measured
557 ATL1415 GL-north tiles on S3, run on the MAAP ADE:
Each worker costs about 0.35 GiB resident. The ADE container is capped at 7.3 GiB, and 8 workers on z0 exceeded it.
Tests
tests/test_mosaic_list.py:block_sizereachingopen_remotemake_mosaic.pyon a remote directory, and its output checkadd_to_bandregression, which fails onmainSuite: 306 passed, 3 skipped.
🤖 Generated with Claude Code