Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 21 additions & 2 deletions src/frame_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,23 @@
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."""
Expand Down Expand Up @@ -968,7 +985,7 @@
pass

_worker_count_used = max(workers, 1)
print(f" Processing {n} frames in parallel ({workers} workers)...")

Check warning on line 988 in src/frame_processor.py

View workflow job for this annotation

GitHub Actions / lint

OS003 8 bare print() call(s) in library code (run --verbose to list, --git to scope to your diff)

# Share calibration arrays via shared memory — zero disk I/O, one copy
# in RAM shared across all workers (read-only view per worker process).
Expand Down Expand Up @@ -1086,9 +1103,11 @@
# 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

Expand Down
16 changes: 16 additions & 0 deletions tests/test_native.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading