Design review report

[kdw503]

---

Phase 4 — Design Review Report

Likely Design Issues

1. Module docstring lists functions from other packages (src/RegisterMismatchCuda.jl:26–35)
   The top-level docstring advertises nanpad, highpass, correctbias!, truncatenoise!, aperture_grid, and allocate_mmarrays as if they live here. None of them are exported or defined in
   this package — they come from companion packages. A first-time user reading the docs would search for these, get a MethodError, and be confused.

2. assertsamesize only checks 3 dimensions (src/RegisterMismatchCuda.jl:328–329)
   size(A,1) == size(B,1) && size(A,2) == size(B,2) && size(A,3) == size(B,3)
   For 1D arrays this silently checks only dimension 1 (which is always checked). For 4D+ arrays it ignores higher dimensions entirely. Should be size(A) == size(B).

3. fftfunc::Any / ifftfunc::Any fields in CMStorage (lines 91–92)
   These fields sit on the hot path — every mismatch! call multiplies by them. Typing them Any suppresses specialization and forces runtime dispatch. They could be typed as
   AbstractFFTs.Plan (or a concrete subtype), or made type parameters on CMStorage.

4. Active deprecations that have not been removed (lines 333–341)
   There are three deprecated callsites:

• CuRCpair(realtype, realsize) → CuRCpair{T}(undef, realsize)
• CMStorage{T}(::UndefInitializer, WidthLike, DimsLike) → tuple form
• CMStorage(::Type{T}, ...) → CMStorage{T}(undef, ...)

These carry maintenance debt and emit runtime warnings to downstream users.

5. cucartesianindex only handles N = 1, 2, 3 (kernels.jl:1–6)
   CMStorage{T,N} is parametric in N, so a user can create 4D storage. But cucartesianindex has no method for N ≥ 4 — any GPU kernel launch on 4D data would fail at runtime with an
   obscure error. This is a latent type-system gap.

---

Design Questions

Q1 — Is CuRCpair genuinely public?
CuRCpair is exported but is purely a GPU r2c buffer layout detail. No user of mismatch or mismatch! ever needs to touch it directly. Was it exported to allow power users to build
custom storage types, or is it an accidental export? If the latter, removing it from the export list would be a minor breaking change.

Q2 — normalization as Symbol vs. dispatch type
Both high-level entry points take normalization=:intensity or :pixels, which routes to one of two completely different GPU kernels via a runtime if. An alternative design would use
singleton dispatch types (struct IntensityNorm end). The symbol approach matches the CPU counterpart in RegisterMismatchCommon — was keeping the API identical a deliberate choice?
Changing it would be breaking.

Q3 — display=false keyword as perpetual compat shim (lines 115–117)
The constructors accept a display keyword that is explicitly ignored "for compatibility with the CPU version." Is this expected to remain indefinitely, or is there a plan to drop it
once users have migrated? It currently silently swallows the argument rather than warning.

Q4 — NanCorrFFTs field names I0, I1, I2
Inside fillfixed!, I1 holds the image, I2 holds the elementwise square, and I0 holds the NaN mask (theta). The index-based names are opaque. Was there a reason not to name them
theta, image, image_sq? (This is internal-only so it's not a breaking change.)

Q5 — mismatch_apertures accepts kwargs... passed to CMStorage (lines 202–213)
The only recognized CMStorage keyword is display=false. Any other keyword silently disappears. Was this intended as a forward-compatibility shim, or should unknown keywords produce
an error?

---

Observations

• calculate_threads, kernel_conv_components!, kernel_calcNumDenom_intensity!, and kernel_calcNumDenom_pixels! are all accessed via RM. in the test suite but have no docstrings and
  no public annotation. If they are intended for advanced users building custom kernels, they warrant docstrings. If they are truly internal, the test-qualified access could be
  replaced.
• CMStorage defines eltype and ndims but not size. A user who wants to know what aperture size a given storage handles must access cms.aperture_width directly. A size(cms) method
  returning the aperture size would be natural.
• mismatch_apertures (line 192) calls CuArray{T}(sdata(fixed)) — sdata is a SharedArrays function for unwrapping shared arrays. This suggests fixed was once expected to commonly be
  a SharedArray. Is this still a realistic input type, or a historical artifact?
• CuRCpair.rng is typed NTuple{N} with no element type constraint. In practice it always holds UnitRange{Int} elements, but the loose typing means the compiler can't specialize on
  it.
• No show methods for CMStorage, CuRCpair, or NanCorrFFTs. At the REPL these fall back to the generic mutable struct display, which prints all fields including raw GPU array
  pointers — not very readable.

---

Overall Character

RegisterMismatchCuda is a tightly scoped, purpose-built GPU backend. Its design is largely coherent: the CMStorage / fillfixed! / mismatch! three-step pattern is clean, and the
allocating vs. in-place function pairs follow standard Julia idioms. The main tensions are (a) a module docstring that describes a richer API than the package actually owns,
creating a misleading first impression; (b) three active deprecations that should have been removed; and (c) fftfunc::Any on the hot path. If the author wanted to address the two
highest-leverage items, they would be: fix the module docstring (zero-cost, high user-facing impact) and remove the deprecations (the TODO: remove deprecations step in the
maintenance workflow will handle this).

---

Phase 5 — Values Clarification

Before writing the plan, I'd like to understand your intent on a few points:

1. Scope and audience: Is this package purely a GPU backend consumed by RegisterMismatch users, or do you intend it to be usable standalone? And is CuRCpair's export intentional —
   are there users who construct them directly?
2. Central abstraction: CMStorage is clearly the centerpiece. Should it stay as-is (a flat mutable struct with all fields public), or would you prefer it to move toward a more
   opaque type over time?
3. Composability / Base interfaces: How much do you want to invest in show, size(cms), etc.? Or is the REPL experience for these types not a priority?
4. Error model: The codebase mixes bare error() and ArgumentError. Should there be a consistent style?
5. normalization as Symbol: Would you consider moving to dispatch types, or does keeping it consistent with the CPU counterpart (RegisterMismatchCommon) take priority?

Take as long as you need — your answers will shape which chunks land in the plan and which get dropped.