Design review report

[kdw503]

---

Phase 3 + 4 — Design Review Report: RegisterMismatch.jl

---

Likely design issues

1. Exported validation guards with no user-facing purpose

checksize_maxshift and assertsamesize are exported but are purely precondition guards for internal call
sequences. A user calling mismatch or fillfixed! has no natural reason to call these themselves; they exist to
produce informative errors inside the package's own code. Exporting them implies a contract ("users may rely on
this API") that the package doesn't actually intend to maintain.

2. tovec — generic name, no domain meaning

tovec(x) converts a tuple or scalar shift specification to a Vector. The name is so generic it will silently
conflict with any other package that defines a similar utility (e.g. LinearAlgebra, geometry packages). It has
no registration-domain meaning. If it needs to be part of the shared interface with RegisterMismatchCuda.jl, it
should either be renamed (e.g. _tovec) or placed in a non-exported namespace.

3. CMStorage type parameters expose implementation artifacts

CMStorage{T, N, RCType, FFT, IFFT} — the last two parameters capture the types of compiled FFTW plan closures,
which vary across platforms and compiler settings. Users cannot write cms::CMStorage{Float64, 2} in their own
code; they must either leave it untyped or write out the full five-parameter signature (which they cannot know
without calling typeof). For a performance-critical package where users are expected to build loops around
fillfixed!/mismatch!, this makes typed dispatch on CMStorage essentially impossible. The FFT/IFFT parameters
are never meaningfully dispatched on; they are captured from construction.

4. mismatch0 is two unrelated operations sharing a name

Overload (a): computes the scalar zero-shift mismatch between two images, returning a NumDenom.
Overload (b): aggregates an entire Array{MismatchArray} of aperture results, returning a NumDenom.

These share only the return type, not their role in any pipeline. Overload (b) is an aperture-aggregation
function; its name suggests "mismatch at zero shift," which is misleading. A user reading mismatch0(mms) where
mms is an aperture array has no reason to guess this computes an integral over all apertures rather than the
zero-shift entry of each one.

---

Design questions

Q1. Are shiftrange, padsize, padranges, default_aperture_width, each_point exported for
RegisterMismatchCuda.jl's benefit?

These functions are geometric utilities internal to the FFT mismatch algorithm: they compute how big the padded
arrays should be, where within a padded array a block sits, etc. They have no natural use in a caller that is
just trying to register two images. If they are exported specifically so RegisterMismatchCuda.jl can call them
without duplicating code, that's a valid reason — but it suggests these functions belong in
RegisterMismatchCommon (or a dedicated RegisterMismatchCommon.Internals submodule), not in RegisterMismatch's
public face. Was the intent to export them as a shared implementation interface, or did they accidentally end
up in the exports?

Q2. Should mismatch_apertures(fixed, moving, gridsize, maxshift) internally call the (aperture_centers,
aperture_width, maxshift) overload?

Currently two overloads of mismatch_apertures exist: one that accepts a gridsize and one that accepts explicit
aperture_centers+aperture_width. If the gridsize form simply computes centers (via aperture_grid) and width
(via default_aperture_width) and calls the second form, the second form becomes the canonical one and code
duplication is eliminated. If instead the two overloads have meaningfully different implementations, what is
the reason?

Q3. Is CMStorage intended to appear in user type signatures?

The package's high-level API (mismatch, mismatch_apertures) hides CMStorage entirely. The low-level API
(fillfixed!, mismatch!, mismatch_apertures!) requires the user to create and hold a CMStorage. For users
writing a typed registration loop, how should they annotate the storage variable? Was there an intention to
provide an abstract type or a narrower alias that users could write (e.g. CMStorage{T,N} with the last three
parameters elided)?

Q4. Is correctbias! part of the recommended pipeline, or an optional advanced correction?

The name and placement don't signal optionality. A user arriving at RegisterMismatch from a tutorial that shows
mismatch → correctbias! → truncatenoise! → register_translate would treat all three post-processing steps as
mandatory. Are these best-effort optional corrections (in which case they might belong in a post-processing
module or package), or are they steps every user should apply? The distinction affects where they live and how
they should be documented.

Q5. Does mismatch_apertures! call fillfixed! internally on every aperture?

The conceptual map notes that mismatch_apertures! calls fillfixed! + mismatch! in a loop over apertures. If so,
the fillfixed!/mismatch! split — which exists to amortize the cost of FFT-transforming the fixed image once
across many moving frames — is not available through mismatch_apertures!. Is there an intended path for users
who want the aperture-based API with a pre-loaded fixed image across multiple moving images?

---

Observations

1. fftnan! is not exported but appears in the test suite. If RegisterMismatchCuda.jl calls it, it does so
   through a non-public name. Either it should be exported (or marked public) as part of the shared interface, or
   it should be annotated clearly as internal-only.
2. allow_inner_threading! / inner_threading modify global state. This is a valid escape hatch for environments
   where Julia's own outer parallelism conflicts with FFTW's inner threads, but it is a global side effect. Users
   who call RegisterMismatch from multiple concurrent tasks with different threading preferences will see
   interference.
3. NanCorrFFTs is unexported but visible in REPL output. When a user prints a CMStorage value, NanCorrFFTs will
   appear in the output if show descends into the struct. This leaks an internal name into user-visible output
   with no documentation.
4. DimsLike and WidthLike improve readability of signatures but are opaque to users. A user writing their own
   wrapper that calls mismatch_apertures needs to know what argument types are accepted. The type aliases are
   helpful for reading the source but their definitions are not user-visible without reading the
   RegisterMismatchCommon source.
5. aperture_range and aperture_grid are genuinely useful for visualization and debugging (e.g., plotting the
   region a given aperture covers). Their presence in the API is reasonable, but their connection to the
   registration pipeline is not obvious from the names alone.
6. The CMStorage{T}(undef, ...) constructor requires an explicit T. Unlike most Julia constructors where
   element type can be inferred, users must write CMStorage{Float64}(undef, ...). This is fine for a
   performance-critical allocation, but worth documenting prominently since it's easy to call CMStorage(undef,
   ...) and get an error with a confusing message.

---

Overall design characterization

RegisterMismatch has a clear identity and a well-motivated central abstraction (CMStorage as a pre-compiled FFT
workspace). The high-level API is clean and the NaN-aware correlation approach is a genuine domain
contribution. The main tension is between two audiences: (a) end users who want mismatch → argmin → done, and
(b) pipeline developers (including RegisterMismatchCuda.jl) who need access to internals for code reuse. Both
groups' needs have been served, but with the side effect that the export list contains a mix of genuinely
user-facing names, implementation-sharing exports meant for sibling packages, and precondition guards that
snuck in. The two highest-leverage changes would be: (1) auditing the export list to either remove or
explicitly document the "shared implementation" exports, and (2) resolving the CMStorage type-parameter problem
so that users who build loops around the low-level API can write typed code.

---

Phase 5 — Values clarification

Before I write the plan, I'd like to hear your answers to a few questions that came up during the review:

1. Scope and audience: Is the primary user a researcher writing a registration pipeline in Julia, or is
   RegisterMismatch primarily a backend library consumed by RegisterMismatchCuda.jl and similar packages? The
   answer affects whether shiftrange, padsize, etc. belong in the exports.
2. Central abstraction: Should CMStorage remain user-visible (users construct it, pass it to fillfixed!, loop
   over mismatch!)? Or should the long-term API trend toward hiding it behind the high-level functions? The
   type-parameter problem is easier to tolerate if the answer is "yes, it's user-facing."
3. Composability / Base relationship: How important is it that CMStorage be writable in typed signatures? Would
   you be comfortable with a public abstract supertype (AbstractCMStorage) or a narrower alias?
4. Error and failure model: What should happen when assertsamesize or checksize_maxshift fail — ArgumentError,
   DimensionMismatch, or something else? Is the current behavior consistent? (I haven't checked the source; just
   flagging this for discussion.)
5. mismatch0 duality: Was the aperture-aggregation overload intentionally named mismatch0, or did it end up
   there because it returns a "zero-level" (scalar) summary of the aperture results? If the latter, a rename like
   aggregate_mismatch or sum_mismatch might be clearer.
6. Release strategy: Some of the findings above (removing exports, renaming mismatch0) would be breaking.