Make target_chunk_mb actually bound memory on the FITS path - #62
Merged
Conversation
added 2 commits
August 20, 2026 09:41
Three unbounded spots in the dask FITS path, all of which made peak memory a function of cube size rather than of target_chunk_mb. _read_fits_block returned a lazy memmap view, so nothing was read there and whoever materialised it faulted in most of the cube. Reads now go through astropy's .section with memmap=False, and spatial chunks are full-width y bands instead of square tiles so each channel's slice of a block is one contiguous run on disk. Blocks come back native-endian, which also drops the astype layer dask was grafting onto every read. _channel_mad_lazy rechunked the spatial axes to a single block, which on a cube with one frequency chunk is the whole cube in one task. Reached eagerly from rmsynth_3d_from_fits for every noise-based weight_type. Per-channel noise now gets its own frequency-chunked read of the same files (read_fits_cube_channel_chunks); the array-based entry point still gathers, but says so. An FDF chunk is n_phi long in complex128 against n_freq in float32, so it outgrew the input chunk it was sized from by a factor of tens. rmsynth_3d now shrinks spatial chunks to keep the output within the caller's budget. Measured on a 48x1024x1024 cube at target_chunk_mb=4: peak RSS 1524 MB before, 129 MB after, and flat in cube size where it used to track it.
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.
Three places on the dask FITS path made peak memory a function of cube size, so
target_chunk_mbwas effectively inert there. The symptom was dask workers stalling or being killed on tasks namedastype-from-value-read_fits_block-concatenate-.... Thatastypeis dask's own, inserted bydask.array.concatenatebecause the reader declared the FITS on-disk dtype>f4; it was just the first task to touch the data, so it absorbed the cost and took the blame.Diagnosis credit: handed over from a session scoped to
flint-crew/flint, which could not push here.1. The reader returned a lazy memmap view
_read_fits_blockdidnp.asarray(memmap_slice), which does not copy. Nothing was read in the task; whoever materialised the block later faulted in most of the cube, and every pending block kept a whole-file mapping alive. Square spatial tiles made it worse: each channel's slice of a tile iscyshort runs scattered across the file.Now: full-width y-band chunks,
memmap=False, andhdul[0].section[...]indexed against the on-disk shape (anintfor degenerate length-1 axes, so a genuinecy == 1band survives wherenp.squeezewould have eaten it). Blocks come back native-endian, which removes theastypelayer.Full traversal of every block of a 703 MiB cube (288 x 800 x 800 f32), 64 MiB block target, one process, peak RSS delta over the post-import baseline. Apple M-series, warm page cache, so these are not directly comparable to the Linux numbers in the original report (which had the same ranking but a wider spread):
.section.sectionSquare tiles are what makes
.sectionslow (241 short reads per channel); with full-width bands each channel is one contiguous read and it is no slower than memmap while staying near 2x the block size.2. Per-channel noise gathered the whole cube
_channel_mad_lazydidcube.rechunk({1: -1, 2: -1}). Axis 0 was already one chunk, so the result was a single chunk holding the entire cube, reached eagerly fromrmsynth_3d_from_fitsfor every noise-basedweight_typeon Q and U together:flint's
RMSynthOptions.weight_typedefaults to"variance", so every flint run hit this.Resizing the rechunk does not help: every input tile feeds every output channel plane, so dask pins the whole cube across it either way. An exact per-channel median simply cannot be computed from spatially chunked blocks. New
read_fits_cube_channel_chunksreads the transposed chunking instead (whole image planes, chunked along frequency, one contiguous read per task), andrmsynth_3d_from_fitsuses it for the noise estimate since it already has the paths.estimate_channel_noise_mad(q, u)still accepts spatially chunked arrays for array-based callers, but now logs a warning naming the size of the gather rather than doing it silently.3. Output chunks outgrew the input chunks they were sized from
target_chunk_mbsized the float32 input footprint, but an FDF chunk isn_philong in complex128 againstn_freqin float32. Measured atn_freq=96, default phi grid (n_phi=989): a 4 MiB input chunk produced a 162 MiB FDF chunk and a 325 MiB RMSF chunk. This turned out to be the dominant term, not a footnote: it accounted for 6.4 GB of peak RSS on a 96 MiB cube.Rather than document it,
rmsynth_3dnow treats the caller's input chunking as the memory budget and shrinks spatial chunks (y only, so blocks stay contiguous) so the complex128 output fits it. Same config after: 7.7 MiB FDF chunks, ~450 MB compute-phase peak, and the write is faster (4.1 s vs 5.6 s) because the tasks fit better. It only ever shrinks: a caller who chunked coarsely on purpose keeps their chunks when the FDF is no larger than the input.End to end
rmsynth_3d_from_fits,weight_type="variance",target_chunk_mb=4.0, 2 dask threads, peak RSS delta over the pre-compute snapshot:mainThe point is the second column being flat: on
mainpeak memory follows the cube, here it follows the chunk.Tests
tests/_dask_memory_worker.pybuilt its inputs withda.from_arrayon an in-memory array and calledrmsynth_3ddirectly, so it never touchedread_fits_cube_daskorestimate_channel_noise_mad. That is how both bugs shipped green.tests/_fits_memory_worker.py+test_fits_path_memory_scales_with_chunk_size_not_cube_size: drivesrmsynth_3d_from_fitson real FITS files with a noise-basedweight_type, at two cube sizes and one chunk target. Fails onmain(1524 MB vs 723 MB for 4x the pixels).ny % cy != 0), andcy == 1.astypekey inread_fits_cube_dask's graph, and a native dtype.132 passed, 1 skipped(from121 passed, 1 skipped). The suite also drops from ~250 s to ~40 s, because the old memory test was spending most of that time on a pathologically large FDF.Workaround for anyone pinned to a release
weight_type: uniformavoids problem 2 entirely, plus worker headroom of about one full cube for problem 1.Not verified
Not smoke tested through flint's
process_rmsynthflow. The fixes are covered by the memory test above, but the numbers here come from rm-lite's own entry points, not from a run under a realdistributedcluster with a nanny memory limit.