-
Notifications
You must be signed in to change notification settings - Fork 18
497 ingestion memory #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
497 ingestion memory #558
Changes from all commits
d27879f
30a6472
a23fb84
7bf7767
942c27e
4274561
6cb0efe
4500bfa
fa60f14
4e35a13
a7ab60e
5d38dbb
f58593b
61fd99d
d4deac7
1532d67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,14 +7,12 @@ | |
| from typing import TYPE_CHECKING | ||
|
|
||
| import numpy as np | ||
| import zarr | ||
|
|
||
| from mdio.constants import UINT32_MAX | ||
| from mdio.core import Dimension | ||
| from mdio.core.serialization import Serializer | ||
| from mdio.core.utils_write import get_constrained_chunksize | ||
|
|
||
| if TYPE_CHECKING: | ||
| import zarr | ||
| from segy.arrays import HeaderArray | ||
| from zarr import Array as ZarrArray | ||
|
|
||
|
|
@@ -65,6 +63,9 @@ def __post_init__(self) -> None: | |
| self.dim_names = tuple(dim.name for dim in self.dims) | ||
| self.shape = tuple(dim.size for dim in self.dims) | ||
| self.ndim = len(self.dims) | ||
| # Prepare attributes for lazy mapping; they will be set in build_map | ||
| self.header_index_arrays: tuple[np.ndarray, ...] = () | ||
| self.num_traces: int = 0 | ||
|
|
||
| def __getitem__(self, item: int) -> Dimension: | ||
| """Get a dimension by index.""" | ||
|
|
@@ -106,47 +107,62 @@ def from_zarr(cls, zarr_root: zarr.Group) -> Grid: | |
| return cls(dims_list) | ||
|
|
||
| def build_map(self, index_headers: HeaderArray) -> None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't understand the need to change in this function and the next. the map and live_mask are compressed zarr arrays in memory and headers are already processed in batch.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was actually the majority of the reason behind the change. The for start in range(0, total_live_traces, batch_size):
end = min(start + batch_size, total_live_traces)
live_dim_indices = []
# Compute indices for the batch
for dim in self.dims[:-1]:
dim_hdr = index_headers[dim.name][start:end]
indices = np.searchsorted(dim, dim_hdr).astype(np.uint32)
live_dim_indices.append(indices)
live_dim_indices = tuple(live_dim_indices)
# Assign trace indices
trace_indices = np.arange(start, end, dtype=np.uint64)
self.map.vindex[live_dim_indices] = trace_indices # <--- This vindexing assignment
self.live_mask.vindex[live_dim_indices] = True
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, if i understood correctly, if we are writing the live mask from distributed tasks (which are chunked same as seismic) it will cause race conditions and corrupt the live mask (which has larger chunks). e.g. multiple workers may write to the same chunk at the same time. Have you checked if live mask is identical before/after this?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are not writing the live mask in a distributed manner, and the Grid.live_mask chunking also differs from the live mask which is written to durable media. I believe this is 100% safe behavior. |
||
| """Build trace mapping and live mask from header indices. | ||
| """Compute per-trace grid coordinates (lazy map). | ||
|
|
||
| Instead of allocating a full `self.map` and `self.live_mask`, this computes, for each trace, | ||
| its integer index along each dimension (excluding the sample dimension) and stores them in | ||
| `self.header_index_arrays`. The full mapping can then be derived chunkwise when writing. | ||
|
|
||
| Args: | ||
| index_headers: Header array containing dimension indices. | ||
| index_headers: Header array containing dimension indices (length = number of traces). | ||
| """ | ||
| # Number of traces in the SEG-Y | ||
| self.num_traces = int(index_headers.shape[0]) | ||
|
|
||
| # For each dimension except the final sample dimension, compute a 1D array of length | ||
| # `num_traces` giving each trace's integer coordinate along that axis (via np.searchsorted). | ||
| # Cast to uint32. | ||
| idx_arrays: list[np.ndarray] = [] | ||
| for dim in self.dims[:-1]: | ||
| hdr_vals = index_headers[dim.name] # shape: (num_traces,) | ||
| coords = np.searchsorted(dim, hdr_vals) # integer indices | ||
| coords = coords.astype(np.uint32) | ||
| idx_arrays.append(coords) | ||
|
|
||
| # Store as a tuple so that header_index_arrays[d][i] is "trace i's index along axis d" | ||
| self.header_index_arrays = tuple(idx_arrays) | ||
|
|
||
| # We no longer allocate `self.map` or `self.live_mask` here. | ||
| # The full grid shape is `self.shape`, but mapping is done lazily per chunk. | ||
|
|
||
| def get_traces_for_chunk(self, chunk_slices: tuple[slice, ...]) -> np.ndarray: | ||
| """Return all trace IDs whose grid-coordinates fall inside the given chunk slices. | ||
|
|
||
| Args: | ||
| chunk_slices: Tuple of slice objects, one per grid dimension. For example, | ||
| (slice(i0, i1), slice(j0, j1), ...) corresponds to a single Zarr chunk | ||
| in index space (excluding the sample axis). | ||
|
|
||
| Returns: | ||
| A 1D NumPy array of trace indices (0-based) that lie within the hyper-rectangle defined | ||
| by `chunk_slices`. If no traces fall in this chunk, returns an empty array. | ||
| """ | ||
| # Determine data type for map based on grid size | ||
| grid_size = np.prod(self.shape[:-1], dtype=np.uint64) | ||
| map_dtype = np.uint64 if grid_size > UINT32_MAX else np.uint32 | ||
| fill_value = np.iinfo(map_dtype).max | ||
|
|
||
| # Initialize Zarr arrays | ||
| live_shape = self.shape[:-1] | ||
| chunks = get_constrained_chunksize( | ||
| shape=live_shape, | ||
| dtype=map_dtype, | ||
| max_bytes=self._INTERNAL_CHUNK_SIZE_TARGET, | ||
| ) | ||
| self.map = zarr.full(live_shape, fill_value, dtype=map_dtype, chunks=chunks) | ||
| self.live_mask = zarr.zeros(live_shape, dtype=bool, chunks=chunks) | ||
|
|
||
| # Calculate batch size | ||
| memory_per_trace_index = index_headers.itemsize | ||
| batch_size = max(1, int(self._TARGET_MEMORY_PER_BATCH / memory_per_trace_index)) | ||
| total_live_traces = index_headers.size | ||
|
|
||
| # Process headers in batches | ||
| for start in range(0, total_live_traces, batch_size): | ||
| end = min(start + batch_size, total_live_traces) | ||
| live_dim_indices = [] | ||
|
|
||
| # Compute indices for the batch | ||
| for dim in self.dims[:-1]: | ||
| dim_hdr = index_headers[dim.name][start:end] | ||
| indices = np.searchsorted(dim, dim_hdr).astype(np.uint32) | ||
| live_dim_indices.append(indices) | ||
| live_dim_indices = tuple(live_dim_indices) | ||
|
|
||
| # Assign trace indices | ||
| trace_indices = np.arange(start, end, dtype=np.uint64) | ||
| self.map.vindex[live_dim_indices] = trace_indices | ||
| self.live_mask.vindex[live_dim_indices] = True | ||
| # Initialize a boolean mask over all traces (shape: (num_traces,)) | ||
| mask = np.ones((self.num_traces,), dtype=bool) | ||
|
|
||
| for dim_idx, sl in enumerate(chunk_slices): | ||
| arr = self.header_index_arrays[dim_idx] # shape: (num_traces,) | ||
| start, stop = sl.start, sl.stop | ||
| if start is not None: | ||
| mask &= arr >= start | ||
| if stop is not None: | ||
| mask &= arr < stop | ||
| if not mask.any(): | ||
| # No traces remain after this dimension's filtering | ||
| return np.empty((0,), dtype=np.uint32) | ||
|
|
||
| # Gather the trace IDs that survived all dimension tests | ||
| return np.nonzero(mask)[0].astype(np.uint32) | ||
|
|
||
|
|
||
| class GridSerializer(Serializer): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the performance implication of this when writing to cloud storage? This will be A LOT of write requests in sequence. We could probably optimize the size of the block iteration (e.g. give a mock array to ChunkIterator with larger virtual chunks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intended to operate on
_TARGET_MEMORY_PER_BATCHsized live masks, which should end up with relatively few write requests.That does bring up the issue that I seem to have accidentally nuked the internal logic to chunk it in the first place though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correction: I confused myself between the internal Grid chunking and the
utils_write.pychunking size. This does leverage the persistent live_mask chunk size appropriately.blockshould be 512MiB as defined byMAX_SIZE_LIVE_MASK.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, makes sense then. can you validate how many write requests it does for a 100GB-sh file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've validated that it only reaches this block once per live mask chunk. For a 100GB file, this is once. Reduced the maximum size of the live mask and saw the appropriate amount of iterations.