Add native macOS CPU platform support - #167
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #167 +/- ##
==========================================
+ Coverage 59.30% 59.42% +0.12%
==========================================
Files 74 75 +1
Lines 11373 11382 +9
Branches 1968 1967 -1
==========================================
+ Hits 6745 6764 +19
+ Misses 3815 3806 -9
+ Partials 813 812 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
nclack
force-pushed
the
platform/macos
branch
from
September 24, 2026 22:38
4dffa70 to
229478d
Compare
This was referenced Sep 24, 2026
nclack
force-pushed
the
platform/macos
branch
from
September 24, 2026 22:53
229478d to
d88df0e
Compare
nclack
added this pull request to stack #170
September 24, 2026 23:07
nclack
added a commit
that referenced
this pull request
Sep 24, 2026
Translated crops in the CPU backend issue one read per chunk in query order, bypassing the read merging and shard ordering used by CUDA. The CPU executor now uses the same merger to combine adjacent or overlapping ranges and distribute reads across shard files, while decoding each distinct chunk once and assembling every requested crop. The merger's read step is now a separate function. The CPU executor calls it directly, so its temporary storage does not include CUDA's per-chunk dispatch records. CUDA still goes through the existing wrapper, and its behavior is unchanged. Chunk offsets map into merged input ranges without changing the decode-worker setting or the two bounded input buffers. Read-plan storage and temporary sorting allocations respect the executor memory limit; temporary pressure from an active batch is retried. Equal shard paths can be separate strings, as they are in CPU prepared plans. The shutdown fix, which drains active reads before releasing plans and their shard paths, is part of #160, so this branch no longer carries its own copy. #160 also adds `tests/test_cpu_executor.c`; this PR extends it. On its own, this PR lowers the number of simultaneous reads when a shard has at least `decode_workers` contiguous chunks. Each merged read then fills an input buffer, so with the defaults 2 reads are in flight instead of 16. #168 restores it: input buffers hold 256 chunks by default, while merged reads still hold at most `decode_workers` chunks. With 32 workers, reads are at most 32 chunks, so a full buffer splits into at least 8 reads. Merge the two together. The regression tests use a reader that records submissions to check merged ranges and shard order, exact output and repeated-chunk reuse, reader saturation, fill-only batches, fills sharing an input buffer with merged reads, FIFO results, retained output, memory admission, and read failure/shutdown. A new coalescer test, `test_equal_path_copies`, covers equal paths with different pointers. Validation at `9385079`: - [CPU CI](https://github.com/nclack/damacy/actions/runs/36065679537): 24/24 CTest entries pass on Ubuntu and 25/25 on macOS, including the new executor regressions; the extension has no CUDA dependencies. - [CUDA-enabled CI](https://github.com/nclack/damacy/actions/runs/36065682194): 36/36 CTest targets pass, plus the Python import check and coverage upload. - [Build](https://github.com/nclack/damacy/actions/runs/36065677255) and the [existing TSan check](https://github.com/nclack/damacy/actions/runs/36065684441) pass. The strict documentation build passes locally. Changed C files pass formatting checks; `git diff --check` passes. No new Turin throughput or storage measurements have been made. This change addresses the scheduling behavior identified in #163; the performance effect and suspected kernel readahead still need measurement. Stacked on #167; the base is `platform/macos`. #168 stacks on this branch. Closes #163. --------- Co-authored-by: Nathan Clack <nclack@biohub.org>
nclack
added a commit
that referenced
this pull request
Sep 24, 2026
Spatial queries can now resolve a fixed output grid against OME-Zarr 0.5 metadata before chunk planning. Resolution chooses a source level using the full affine sampling scale, including rotation and anisotropy. Aligned crops run on either CPU or CUDA; queries that still require resampling fail explicitly when submitted. This PR does not implement a resampler. Load an immutable `NgffImage` through an injected metadata reader, then call `image.resolve(query, shape=output.shape)`. Resolution performs no I/O and depends only on the requested sample shape. Batching, output allocation, and dtype conversion remain downstream. Each query carries its full-rank transform, sampler, boundary behavior, and automatic or explicit level choice. Results own their source URI and geometry independently of the image. Python returns one immutable, serializable value that can be pushed directly. C fills a caller-owned `damacy_spatial_resolution` with directly readable fields and a `clear()` operation; the existing sample conversion is the compatibility bridge to C submission. Public coordinates use reference-level voxel corners, with identity matching integer crops. NGFF center coordinates are converted at metadata loading. Loading also removes floating-point rounding error from each level's scale ratio and origin, so crops built from the reported level values are exact; the resolver itself never rounds a query into a crop. Validation checks consumed fields needed for array layout and coordinate interpretation, including ambiguous duplicate fields and finite transforms. Unused metadata, shared transforms that cancel in reference coordinates, and trailing content are left uninterpreted. Loading caps each `zarr.json` read at the part of `NgffLimits.max_metadata_bytes` still unused; a larger file fails with `DAMACY_BUDGET`. Whole-file metadata reads, capped or not, also fail that way above `UINT32_MAX` bytes. On macOS the metadata reader is the POSIX worker pool from #167, which had no capped read; this PR adds one there with the same behavior as Linux. See the [spatial query API and contracts](https://github.com/nclack/damacy/blob/ngff-resolution/docs/spatial.md) for C/Python examples, coordinate conversion, level selection, ownership, and the next executor operation. Stacked on #161 (`indexed-queries`). Validation: - CI at `382d921`: the [CPU workflow](https://github.com/nclack/damacy/actions/runs/36066539561) passes 26/26 CTest entries on Ubuntu and 27/27 on [macOS](https://github.com/nclack/damacy/actions/runs/36066539561/job/107857506786), where the metadata budget tests use the new capped read. [CUDA](https://github.com/nclack/damacy/actions/runs/36066541831) passes 38/38, including the native and Python spatial tests. [TSan](https://github.com/nclack/damacy/actions/runs/36066544271) passes 2/2, the prefetcher and spatial tests, and the [build](https://github.com/nclack/damacy/actions/runs/36066537440) passes. - Before the review fixes, CPU: 25/25 CTest targets passed; spatial Python tests: 62 passed, 7 CUDA cases skipped. - Before the review fixes, L40: native spatial tests and all 69 Python spatial tests passed; all 7 CUDA memory-check cases passed with zero sanitizer errors. - Before the review fixes, ASan/UBSan: JSON and spatial tests passed, including C result ownership and cleanup. - Ruff, Pyright for the package and spatial tests, strict documentation build, and C example syntax passed. - Regression coverage includes NGFF center/corner conversion, anisotropic level selection, interpolation bounds, permissive handling of unused metadata, invalid consumed fields, result serialization and ownership, fixed output shapes, 128 collapsed transforms, large differences in sampling scale, floating-point rounding in level scales and translations, unrelated keys that do not decode, and image comparison and copies. --------- Co-authored-by: Nathan Clack <nclack@biohub.org>
github-actions Bot
added a commit
that referenced
this pull request
Sep 24, 2026
Spatial queries can now resolve a fixed output grid against OME-Zarr 0.5 metadata before chunk planning. Resolution chooses a source level using the full affine sampling scale, including rotation and anisotropy. Aligned crops run on either CPU or CUDA; queries that still require resampling fail explicitly when submitted. This PR does not implement a resampler. Load an immutable `NgffImage` through an injected metadata reader, then call `image.resolve(query, shape=output.shape)`. Resolution performs no I/O and depends only on the requested sample shape. Batching, output allocation, and dtype conversion remain downstream. Each query carries its full-rank transform, sampler, boundary behavior, and automatic or explicit level choice. Results own their source URI and geometry independently of the image. Python returns one immutable, serializable value that can be pushed directly. C fills a caller-owned `damacy_spatial_resolution` with directly readable fields and a `clear()` operation; the existing sample conversion is the compatibility bridge to C submission. Public coordinates use reference-level voxel corners, with identity matching integer crops. NGFF center coordinates are converted at metadata loading. Loading also removes floating-point rounding error from each level's scale ratio and origin, so crops built from the reported level values are exact; the resolver itself never rounds a query into a crop. Validation checks consumed fields needed for array layout and coordinate interpretation, including ambiguous duplicate fields and finite transforms. Unused metadata, shared transforms that cancel in reference coordinates, and trailing content are left uninterpreted. Loading caps each `zarr.json` read at the part of `NgffLimits.max_metadata_bytes` still unused; a larger file fails with `DAMACY_BUDGET`. Whole-file metadata reads, capped or not, also fail that way above `UINT32_MAX` bytes. On macOS the metadata reader is the POSIX worker pool from #167, which had no capped read; this PR adds one there with the same behavior as Linux. See the [spatial query API and contracts](https://github.com/nclack/damacy/blob/ngff-resolution/docs/spatial.md) for C/Python examples, coordinate conversion, level selection, ownership, and the next executor operation. Stacked on #161 (`indexed-queries`). Validation: - CI at `382d921`: the [CPU workflow](https://github.com/nclack/damacy/actions/runs/36066539561) passes 26/26 CTest entries on Ubuntu and 27/27 on [macOS](https://github.com/nclack/damacy/actions/runs/36066539561/job/107857506786), where the metadata budget tests use the new capped read. [CUDA](https://github.com/nclack/damacy/actions/runs/36066541831) passes 38/38, including the native and Python spatial tests. [TSan](https://github.com/nclack/damacy/actions/runs/36066544271) passes 2/2, the prefetcher and spatial tests, and the [build](https://github.com/nclack/damacy/actions/runs/36066537440) passes. - Before the review fixes, CPU: 25/25 CTest targets passed; spatial Python tests: 62 passed, 7 CUDA cases skipped. - Before the review fixes, L40: native spatial tests and all 69 Python spatial tests passed; all 7 CUDA memory-check cases passed with zero sanitizer errors. - Before the review fixes, ASan/UBSan: JSON and spatial tests passed, including C result ownership and cleanup. - Ruff, Pyright for the package and spatial tests, strict documentation build, and C example syntax passed. - Regression coverage includes NGFF center/corner conversion, anisotropic level selection, interpolation bounds, permissive handling of unused metadata, invalid consumed fields, result serialization and ownership, fixed output shapes, 128 collapsed transforms, large differences in sampling scale, floating-point rounding in level scales and translations, unrelated keys that do not decode, and image comparison and copies. --------- Co-authored-by: Nathan Clack <nclack@biohub.org> 6e87e2b
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
1 participant
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.
CPU builds currently depend on Linux io_uring and NUMA interfaces. Add native macOS CPU support, with CUDA defaulting off on macOS. macOS builds the same POSIX platform code as Linux, plus a small
numa.darwin.cstub because macOS has no NUMA or CPU affinity controls. Remove the unusedplatform_available_memory, which relied on_SC_AVPHYS_PAGES, a value macOS does not define.Metadata reads use a POSIX worker pool on macOS (
metadata_store_async.posix.c), with the existing async callbacks, cache integration, latency metrics, and drain-on-shutdown behavior. Linux keeps its io_uring backend. Both backends share their metrics, injected latency, and stats code inmetadata_store_async_common.c, so each backend file holds only its request handling. Add metadata lifecycle/error tests, macOS platform checks, native macOS CPU CI, and build documentation.On macOS, metadata callbacks run on several worker threads at once. Every caller is safe with that:
array_meta.c,shard_index.c, andchunk_layout.ceach touch only their own request state, plusprefetch_cache_complete, which takes a lock.The file reader now rejects more workers than online CPUs with
DAMACY_INVALand a log message namingn_io_threadsand the CPU count. Before, the I/O queue refused them and creation reportedDAMACY_OOM. This applies on Linux too. The same limit is whytest_io_queuenow runs its ordering test with two workers instead of four: the hosted macOS runner has three vCPUs.Targets
main. The CPU-only build it relies on came from #160, now merged. #164 stacks on this branch.Validation:
229478d: the CPU workflow passes 25/25 CTest entries on macOS 26 and 24/24 on Ubuntu 24.04. Its dependency check finds no CUDA or liburing in the macOS extension. CUDA passes 36/36, TSan passes, and the build passes.