Skip to content
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 58 additions & 7 deletions src/mdio/converters/segy.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,13 +394,19 @@ def segy_to_mdio( # noqa: PLR0913, PLR0915
grid_density_qc(grid, num_traces)
grid.build_map(index_headers)

# Check grid validity by comparing trace numbers
if np.sum(grid.live_mask) != num_traces:
# Check grid validity by ensuring every trace's header-index is within dimension bounds
valid_mask = np.ones(grid.num_traces, dtype=bool)
for d_idx in range(len(grid.header_index_arrays)):
coords = grid.header_index_arrays[d_idx]
valid_mask &= (coords < grid.shape[d_idx])
valid_count = int(np.count_nonzero(valid_mask))
if valid_count != num_traces:
for dim_name in grid.dim_names:
dim_min, dim_max = grid.get_min(dim_name), grid.get_max(dim_name)
dim_min = grid.get_min(dim_name)
dim_max = grid.get_max(dim_name)
logger.warning("%s min: %s max: %s", dim_name, dim_min, dim_max)
logger.warning("Ingestion grid shape: %s.", grid.shape)
raise GridTraceCountError(np.sum(grid.live_mask), num_traces)
raise GridTraceCountError(valid_count, num_traces)
Comment on lines +397 to +416

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this use more memory? grid.live_mask is a compressed bool zarr array in memory, but we just allocated the full size with np.ones. Doesn't this increase memory consumption?

@BrianMichell BrianMichell Jun 3, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does use more memory in the main process compared to what is live in 0.9.2 (below), but it has a lower peak memory consumption. This should only account for a few megabytes though. I'll look into any other low-hanging fruit as far as memory consumption.

image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may become problematic if like mask is 4-5GB. We can probably still store it as a zarr array.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, valid_mask is freed immediately following the validation check (and I've eagerly collected it as well now). I don't believe it is necessary to hold the compressed mask at all.


if chunksize is None:
dim_count = len(index_names) + 1
Expand Down Expand Up @@ -446,13 +452,58 @@ def segy_to_mdio( # noqa: PLR0913, PLR0915
data_array = data_group[f"chunked_{suffix}"]
header_array = meta_group[f"chunked_{suffix}_trace_headers"]

# Write actual live mask and metadata to empty MDIO
meta_group["live_mask"][:] = grid.live_mask[:]
nonzero_count = np.count_nonzero(grid.live_mask)
live_mask_array = meta_group["live_mask"]
# 'live_mask_array' has the same first N–1 dims as 'grid.shape[:-1]'
# Build a ChunkIterator over the live_mask (no sample axis)
from mdio.core.indexing import ChunkIterator

chunker = ChunkIterator(live_mask_array, chunk_samples=False)
for chunk_indices in chunker:
# chunk_indices is a tuple of N–1 slice objects
trace_ids = grid.get_traces_for_chunk(chunk_indices)
if trace_ids.size == 0:
continue

# Build a temporary boolean block of shape = chunk shape
block_shape = tuple(sl.stop - sl.start for sl in chunk_indices)
block = np.zeros(block_shape, dtype=bool)

# Compute local coords within this block for each trace_id
local_coords: list[np.ndarray] = []
for dim_idx, sl in enumerate(chunk_indices):
hdr_arr = grid.header_index_arrays[dim_idx]
# Optimize memory usage: hdr_arr and trace_ids are already uint32,
# sl.start is int, so result should naturally be int32/uint32.
# Avoid unnecessary astype conversion to int64.
indexed_coords = hdr_arr[trace_ids] # uint32 array
local_idx = indexed_coords - sl.start # remains uint32
# Only convert dtype if necessary for indexing (numpy requires int for indexing)
if local_idx.dtype != np.intp:
local_idx = local_idx.astype(np.intp)
local_coords.append(local_idx)

# Mark live cells in the temporary block
block[tuple(local_coords)] = True

# Write the entire block to Zarr at once
live_mask_array.set_basic_selection(selection=chunk_indices, value=block)

Copy link
Copy Markdown
Contributor

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)

Copy link
Copy Markdown
Collaborator Author

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_BATCH sized 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.

Copy link
Copy Markdown
Collaborator Author

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.py chunking size. This does leverage the persistent live_mask chunk size appropriately. block should be 512MiB as defined by MAX_SIZE_LIVE_MASK.

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Collaborator Author

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.


nonzero_count = grid.num_traces

write_attribute(name="trace_count", zarr_group=root_group, attribute=nonzero_count)
write_attribute(name="text_header", zarr_group=meta_group, attribute=text_header.split("\n"))
write_attribute(name="binary_header", zarr_group=meta_group, attribute=binary_header.to_dict())

# Clean up live mask related variables
del live_mask_array
del chunker
del block
del local_coords
del trace_ids
del chunk_indices
import gc
gc.collect()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't necessarily have to do all this; can you profile it end-to-end without this and see if its necessary

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deeper dive into why I did this

Here's a zoomed in view of commit 4274561b4eebab1894509d17d7520d74e5fa87d2 (No eager del or gc)
image

Here's a zoomed in view after adding the eager garbage collection
image

These two plots show the period between from mdio.core.indexing import ChunkIterator and # Clean up live mask related variables in the current code.

It helped decrease heap allocation by ~200MiB to eagerly delete and garbage collect the objects.

*I failed to drop the cache on the After run of the initial PR message. These figures are both with dropped cache.

# Write traces
stats = blocked_io.to_zarr(
segy_file=segy,
Expand Down
96 changes: 58 additions & 38 deletions src/mdio/core/grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,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."""
Expand Down Expand Up @@ -106,47 +109,64 @@ def from_zarr(cls, zarr_root: zarr.Group) -> Grid:
return cls(dims_list)

def build_map(self, index_headers: HeaderArray) -> None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually the majority of the reason behind the change.

The vindex used to eagerly generate the map appeared to be responsible for the sharp spike around 11:05 in the initial figure. The intent of these changes was to effectively defer those computations to the distributed tasks.

        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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The 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 final sample dimension) and stores them in
`self.header_index_arrays`. The full mapping can then be derived chunk-by-chunk 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.
return

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
trace_ids = np.nonzero(mask)[0].astype(np.uint32)
return trace_ids


class GridSerializer(Serializer):
Expand Down
68 changes: 40 additions & 28 deletions src/mdio/segy/_workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,45 +77,57 @@ def trace_worker(
Returns:
Partial statistics for chunk, or None
"""
# Special case where there are no traces inside chunk.
live_subset = grid.live_mask[chunk_indices[:-1]]

if np.count_nonzero(live_subset) == 0:
# Determine which trace IDs fall into this chunk
trace_ids = grid.get_traces_for_chunk(chunk_indices[:-1])
if trace_ids.size == 0:
return None

# Let's get trace numbers from grid map using the chunk indices.
seq_trace_indices = grid.map[chunk_indices[:-1]]

tmp_data = np.zeros(seq_trace_indices.shape + (grid.shape[-1],), dtype=data_array.dtype)
tmp_metadata = np.zeros(seq_trace_indices.shape, dtype=metadata_array.dtype)

del grid # To save some memory

# Read headers and traces for block
valid_indices = seq_trace_indices[live_subset]

traces = segy_file.trace[valid_indices.tolist()]
# Read headers and traces for the selected trace IDs
traces = segy_file.trace[trace_ids.tolist()]
headers, samples = traces["header"], traces["data"]

tmp_metadata[live_subset] = headers.view(tmp_metadata.dtype)
tmp_data[live_subset] = samples

# Flush metadata to zarr
# Build a temporary buffer for data and metadata for this chunk
chunk_shape = tuple(sli.stop - sli.start for sli in chunk_indices[:-1]) + (grid.shape[-1],)
tmp_data = np.zeros(chunk_shape, dtype=data_array.dtype)
meta_shape = tuple(sli.stop - sli.start for sli in chunk_indices[:-1])
tmp_metadata = np.zeros(meta_shape, dtype=metadata_array.dtype)

# Compute local coordinates within the chunk for each trace
local_coords: list[np.ndarray] = []
for dim_idx, sl in enumerate(chunk_indices[:-1]):
hdr_arr = grid.header_index_arrays[dim_idx]
# Optimize memory usage: hdr_arr and trace_ids are already uint32,
# sl.start is int, so result should naturally be int32/uint32.
# Avoid unnecessary astype conversion to int64.
indexed_coords = hdr_arr[trace_ids] # uint32 array
local_idx = indexed_coords - sl.start # remains uint32
# Only convert dtype if necessary for indexing (numpy requires int for indexing)
if local_idx.dtype != np.intp:
local_idx = local_idx.astype(np.intp)
local_coords.append(local_idx)
full_idx = tuple(local_coords) + (slice(None),)

# Populate the temporary buffers
tmp_data[full_idx] = samples
tmp_metadata[tuple(local_coords)] = headers.view(tmp_metadata.dtype)

# Flush metadata to Zarr
metadata_array.set_basic_selection(selection=chunk_indices[:-1], value=tmp_metadata)

# Determine nonzero samples and early-exit if none
nonzero_mask = samples != 0
nonzero_count = nonzero_mask.sum(dtype="uint32")

nonzero_count = int(nonzero_mask.sum())
if nonzero_count == 0:
return None

# Flush data to Zarr
data_array.set_basic_selection(selection=chunk_indices, value=tmp_data)

# Calculate statistics
tmp_data = samples[nonzero_mask]
chunk_sum = tmp_data.sum(dtype="float64")
chunk_sum_squares = np.square(tmp_data, dtype="float64").sum()
min_val = tmp_data.min()
max_val = tmp_data.max()
flattened_nonzero = samples[nonzero_mask]
chunk_sum = float(flattened_nonzero.sum(dtype="float64"))
chunk_sum_squares = float(np.square(flattened_nonzero, dtype="float64").sum())
min_val = float(flattened_nonzero.min())
max_val = float(flattened_nonzero.max())

return nonzero_count, chunk_sum, chunk_sum_squares, min_val, max_val
return (nonzero_count, chunk_sum, chunk_sum_squares, min_val, max_val)
Loading
Loading