diff --git a/CHANGELOG.md b/CHANGELOG.md index e821124..2117da3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ match the `VERSION` file and `v*` git tags. ### Changed +- **`--use-gpu` real-hardware finding: currently a net loss on a 4GB card for real sessions, not just a + hypothesis.** Measured head-to-head on the same real session, same machine: 54.4s with `--use-gpu` vs + ~37s CPU-only. Root cause is mostly architectural, not a quick fix: Phase 1's GPU-mode worker count is + capped by VRAM (~7 workers on a 4GB card vs 16 CPU-mode workers), while most of Phase 1's actual + work (lacosmic, debayer, star detection) stays CPU-native regardless of `--use-gpu` -- only + calibration/hot-pixel/white-balance dispatch to GPU, so the lost worker parallelism isn't compensated. + A smaller, real, *fixed* contributor: the GPU-mode quality-metrics thread pool ran uncapped + (`os.cpu_count()` threads) concurrently with the GPU worker threads, neither pinned to a single + rayon/BLAS thread the way CPU mode's process-pool workers are -- real oversubscription, now capped + (`_gpu_quality_pool_size`) so the two pools' thread counts stay near the physical core budget together. + Recovered ~0.7s of the ~17s gap -- real, but confirms the worker-count architecture is the dominant + cause, not thread pinning. Widening GPU coverage further (e.g. to warp/drizzle) was considered and set + aside: it would add more per-worker VRAM demand, shrinking the already-constraining worker count + further rather than fixing it. `--use-gpu` stays opt-in; this is documented in CLAUDE.md for anyone + picking the architectural question back up. + - **The native Gaussian blur kernel now covers per-channel `(H, W, C)` blurs, not just single 2-D planes.** New `gaussian_blur_spatial` (`src/background.py`) blurs each channel independently by looping the existing native kernel per channel -- the same per-channel-native-call pattern this project's diff --git a/CLAUDE.md b/CLAUDE.md index a1f2068..f02ceab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -169,7 +169,7 @@ The main output FITS is the linear pre-post-processing stack (`RAWSTACK=True`) w Frames are processed one at a time: load → process → accumulate → free. Memory usage stays at ~1-2 frames regardless of total frame count. This is the core design constraint — never accumulate all frames in memory. ### Key classes and singletons -- **`GpuContext`** — CPU/GPU abstraction; exposes `xp` (numpy or cupy), `xndimage`, `xsignal`. Accessed via module-level `get_gpu()` in `src/gpu_context.py`. GPU path is opt-in via `--use-gpu`. +- **`GpuContext`** — CPU/GPU abstraction; exposes `xp` (numpy or cupy), `xndimage`, `xsignal`. Accessed via module-level `get_gpu()` in `src/gpu_context.py`. GPU path is opt-in via `--use-gpu`. **On a real 4GB card, `--use-gpu` currently measures *slower* than CPU-only for real-shaped sessions** (measured head-to-head on the same real 15-frame session, same machine: 54.4s vs ~37s CPU-only) -- not a hypothesis, a real run. Root cause: Phase 1's GPU-mode worker count is capped by VRAM (`max_gpu_workers`, `Config.GPU_PHASE1_WORKER_MB`=450MB/worker -> ~7 workers on a 4GB card after `GPU_VRAM_RESERVE_MB`), well under CPU-mode's `os.cpu_count()` (16 here), while most of Phase 1's actual work (lacosmic, debayer interpolation, CFA equalization, star detection) stays CPU-native regardless of `--use-gpu` -- only calibration/hot-pixel/white-balance dispatch to GPU at all, so the lost worker-count parallelism isn't compensated by GPU throughput on the steps that do run there. A secondary, smaller factor: `execute_frame_processing`'s GPU-mode quality-metrics thread pool (`_qpool`, CPU-bound star detection/FWHM run concurrently with the GPU workers so they're never idle) used to run at a full uncapped `os.cpu_count()` threads *on top of* the GPU workers, and neither pool is rayon/BLAS-thread-pinned the way CPU mode's `ProcessPoolExecutor` workers are (that pinning is subprocess-initializer-only, and this is all one process under GPU mode) -- real oversubscription, but capping the quality pool to `os.cpu_count() - n_workers` only recovered ~0.7s of the ~17s gap, so it's real but not the dominant cause. **Widening GPU coverage further (e.g. to warp/drizzle) was considered and set aside**: it would add more per-worker VRAM demand, shrinking the already-constraining worker count further, compounding this problem rather than fixing it. The actual fix would be architectural (decouple Phase 1 worker count from VRAM when the GPU-dispatched fraction of work is small, or reduce `GPU_PHASE1_WORKER_MB`'s footprint) and wasn't attempted -- real risk of OOM on other users' cards if the budget is cut wrong, and no evidence yet on whether it would pay off on a larger card. `--use-gpu` stays opt-in and this finding is not yet reflected in user-facing docs beyond this file. - **`Config`** — Central class for magic number constants (thresholds, defaults) in `src/models.py`. - **`FrameInfo`** — Dataclass holding path, type, header, quality metrics, and computed shift for each frame. - Master calibration frames (bias/dark/flat) are plain numpy arrays stored in a `masters` dict passed through the pipeline. diff --git a/src/frame_processor.py b/src/frame_processor.py index 8d8d304..3abc13d 100644 --- a/src/frame_processor.py +++ b/src/frame_processor.py @@ -665,6 +665,23 @@ def _pin_worker_to_single_thread() -> None: os.environ['RAYON_NUM_THREADS'] = str(_RAYON_WORKER_CAP) +def _gpu_quality_pool_size(n_workers: int, cpu_count: int, n_frames: int) -> int: + """Thread count for ``execute_frame_processing``'s GPU-mode quality pool. + + Runs concurrently with ``n_workers`` GPU threads in the same process -- + neither pool is rayon/BLAS-thread-pinned the way CPU mode's + ``ProcessPoolExecutor`` workers are (that pinning only runs in a fresh + subprocess's own initializer, which a ``ThreadPoolExecutor`` never + spawns), so an unbounded quality pool stacks a full ``cpu_count`` more OS + threads on top of the GPU workers, each independently capable of + launching full-core-count native rayon parallelism for its own + (still-CPU-only) work -- real oversubscription, measured on a real + 4GB-card session. Capped so the two pools' thread counts together stay + near the physical core budget instead of stacking on top of each other. + """ + return max(1, min(cpu_count - n_workers, n_frames)) + + def _banding_cfg(args) -> Optional[tuple]: """(amount, sigma) when --banding-removal is on, else None. Carried to pool workers through the initializer (like trail_reject), not the task tuple.""" @@ -1086,9 +1103,11 @@ def _accum(timings: Optional[dict]) -> None: # When GPU is active, quality metrics (CPU-bound: star detection, FWHM) run # in a dedicated CPU pool so GPU workers are never idle waiting for them. # GPU workers skip quality (skip_quality=True) and return immediately after - # writing to memmap, keeping VRAM freed as quickly as possible. + # writing to memmap, keeping VRAM freed as quickly as possible. Pool size + # is capped -- see _gpu_quality_pool_size's docstring. _use_qpool = gpu.active - _n_cpu = min(os.cpu_count() or 4, n) + _n_cpu = (_gpu_quality_pool_size(n_workers, os.cpu_count() or 4, n) if _use_qpool + else min(os.cpu_count() or 4, n)) _qpool = ThreadPoolExecutor(max_workers=_n_cpu) if _use_qpool else None _qfuts: Dict[int, Any] = {} # frame index → quality Future diff --git a/tests/test_native.py b/tests/test_native.py index 593ee8c..4a15d7c 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -2629,3 +2629,19 @@ def test_rotated_warp_weight_table_matches_closed_form(): assert (a == b).mean() > 0.99 ulp = np.spacing(np.abs(outs["exact"]).astype(np.float32)).astype(np.float64) assert (np.abs(a - b) <= 4 * ulp + 1e-3).all() + + +def test_gpu_quality_pool_size_stays_within_core_budget(): + """execute_frame_processing's GPU-mode quality-thread-pool sizing -- + see _gpu_quality_pool_size's own docstring for why this is capped at + all (real measured oversubscription on a 4GB card).""" + import src.frame_processor as fp + + # GPU workers + quality pool should never together demand more than + # the physical core budget. + assert fp._gpu_quality_pool_size(n_workers=7, cpu_count=16, n_frames=100) == 9 + # Never below 1, even if GPU workers alone already meet or exceed cpu_count. + assert fp._gpu_quality_pool_size(n_workers=16, cpu_count=16, n_frames=100) == 1 + assert fp._gpu_quality_pool_size(n_workers=20, cpu_count=16, n_frames=100) == 1 + # Never more threads than there are frames to process. + assert fp._gpu_quality_pool_size(n_workers=2, cpu_count=16, n_frames=3) == 3