Make v2→v3 re-encode safer: prefix-safe omit_nodes and lower peak memory#1
Make v2→v3 re-encode safer: prefix-safe omit_nodes and lower peak memory#1wietzesuijker wants to merge 2 commits into
Conversation
|
|
||
| chunk_shape = tuple(new_array.metadata.chunk_grid.chunk_shape) | ||
| # Iterate using the destination chunk grid to bound peak memory. | ||
| regions_iter = _iter_chunk_regions(tuple(new_array.shape), chunk_shape) |
There was a problem hiding this comment.
we can use the _iter_shard_regions method defined on the new array, it's private API in zarr but it's not likely to change
There was a problem hiding this comment.
if we don't want to depend on private API and instead want to use our own version, we need to implement tests
|
hi @wietzesuijker thanks for this! |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
| existing = root_attrs.get("_eopf_geozarr") | ||
| eopf_meta: dict[str, object] = dict(existing) if isinstance(existing, dict) else {} | ||
| eopf_meta["omit_nodes"] = sorted(omit_nodes) | ||
| root_attrs["_eopf_geozarr"] = eopf_meta |
There was a problem hiding this comment.
why do we need to store the names of missing groups as metadata?
There was a problem hiding this comment.
asking because, presuming we have access to the original data, we can find the names of omitted groups by checking against the original data
this will use extra IO if the source chunks are misaligned to the destination chunks, because the same source chunk will be read multiple times. But if the source array fits in memory, eagerly fetching it reads each chunk exactly once, so I think that's a better approach until we have an actual memory budget problem. |
wietzesuijker
force-pushed
the
pr-92
branch
8 times, most recently
from
04a3290 to
da6a42d
Compare
wietzesuijker
changed the title
wietzesuijker
changed the title
| # If chunking differs, writing by destination chunk regions can re-read source chunks. | ||
| # Prefer eager copy for smaller arrays to minimize IO; fall back to chunk-by-chunk when | ||
| # the array is too large to comfortably materialize. | ||
| eager_copy_max_bytes = 256 * 1024 * 1024 |
There was a problem hiding this comment.
i do not understand the reasoning for this optimization. what memory problem are we solving here?
|
Sorry let's discard |
3 participants
Aims to make the v2→v3 re-encode path a bit safer and easier to operate, without changing its intent (still a re-encode, not a raw-bytes copy). The changes focus on reducing memory spikes during copying, making “omit” behavior reproducible, and avoiding private APIs that could break on dependency upgrades.
What changed (and why):
omit_nodesis normalized and subtree-safe (inzarrio.reencode_group) sofoowon’t accidentally matchfoobar; the normalized omit list is recorded in output root attrs (_eopf_geozarr.omit_nodes) to make results easier to audit and reproduce.new_array[...] = old_array[...], which lowers peak memory and is typically friendlier to object stores.xarray/zarrhelpers; it uses small local routines for time-unit mapping and chunk/shard sizing to reduce “upgrade surprise” risk.add_multiscales_metadata_to_parentis annotated asDataTree | None, matching its real skip behavior.@d-v-b, hope this is somewhat helpful / feel free to cherry-pick or discard