Design review report

[kdw503]

---

Phase 3+4 — Design Review Report: RegisterOptimize

---

Likely Design Issues

1. fit_sigmoid is exported but is an internal subroutine.
   fit_sigmoid fits a logistic curve to penalty data — its documented and apparent sole use is as a subroutine inside auto_λ. Exporting it implies it's public API with independent use cases, but none are documented. It's unusual for a
   helper this specific to be exported.

2. rotation_gridsearch and grid_rotations are documented but not exported.
   The module-level docstring lists rotation_gridsearch as a main function, yet it is not exported, forcing users to write RegisterOptimize.rotation_gridsearch(...). This is inconsistent: the package documents a capability as top-level but
   gates access behind qualified syntax.

3. optimize! is architecturally central but not exported.
   optimize! is the core nonlinear optimization step for deformable registration. It has a docstring, is called in tests via qualified access (RegisterOptimize.optimize!), and is used by fixed_λ. Users working at the intermediate level
   (they have an initial guess and want to optimize) have no public entry point. Either it should be exported/marked public, or its role should be explicitly reserved for internal use and its signature can be simplified.

4. InitialDefOpt is dead code.
   The conceptual mapper notes this type is "broken/unused" — it exists as a mutable struct <: GradOnlyBoundsOnly but the code path that uses it (ϕ_old as GridDeformation) is nonfunctional. Dead internal types accumulate maintenance burden.

5. GradOnly has exactly one concrete subtype (GradOnlyBoundsOnly), making it a speculative abstraction.
   GradOnly → GradOnlyBoundsOnly → {RigidOpt, DeformOpt, DeformTseriesOpt, InitialDefOpt} has an extra layer with no current dispatch value. If InitialDefOpt is removed, this hierarchy could flatten without loss.

6. auto_λ returns a 6-tuple.
   (ϕ_best, penalty, λ_best, λ_all, datapenalty_all, quality) is hard to work with: positional unpacking is fragile, and passing the result to another function requires manual decomposition. A NamedTuple or lightweight result struct would
   make this much more composable.

---

Design Questions

Q1. Is optimize! intended for external callers?
It has a docstring and is qualified-called in tests. If yes → export it or annotate public. If no → its three-return (ϕ, fval, fval0) is an internal detail that can be simplified.

Q2. Should rigid and deformable registration live in the same package?
optimize_rigid / rotation_gridsearch / grid_rotations form a self-contained rigid-registration sub-API. They share no types with the deformable side. Was the intent "all optimization in one package," or could a future RegisterRigid.jl
split make the scope of each cleaner?

Q3. What is the relationship between auto_λt and auto_λ?
auto_λt uses only a quadratic approximation to estimate λt (not the full optimizer), while auto_λ runs the full optimizer for spatial λ. Is this a deliberate speed tradeoff, or is a full-optimization auto_λt a planned but absent
function?

Q4. Is the BoundsOnly abstract type load-bearing?
SigmoidOpt is its only concrete subtype. Was this set up speculatively (expecting other sigmoid-like problems) or was there a concrete reason to keep it separate from GradOnlyBoundsOnly?

Q5. Should the adaptive mu_init fallback in fixed_λ be silent?
When mu_init is too large, fixed_λ silently retries with smaller values (down to 1e-16). Is hiding this from the caller intentional (simplicity), or should there be a warning/log so users know their problem is ill-conditioned?

Q6. Is the RigidValue/RigidOpt split necessary?
RigidValue evaluates the objective; RigidOpt wraps it as a MOI evaluator. Could RigidOpt absorb RigidValue, or is the separation valuable (e.g., for testing the objective function in isolation)?

---

Observations

• AffineQHessian and TimeHessian implement a clean implicit-operator pattern (*, mul!) used in tests via qualified access. Advanced users extending the CG-based initialization path would benefit from these being public.
• The three-overload layering of auto_λ (image inputs → precomputed mismatch → with stack indexing) is a reasonable progressive API but is not documented as such — users may not discover the lower-level entry points.
• The two-solver strategy (Ipopt for single-frame, Optim/LBFGS for multi-frame) is an implementation detail that leaks through the non-exported optimize! API, whose two methods take different penalty types and return different values.

---

Overall Characterization

RegisterOptimize has a clear purpose and its core algorithms are well-engineered: the implicit CG operator for globally-optimal initialization (AffineQHessian) and the MOI evaluator functor pattern are both solid choices. The main
tension is between internal completeness (everything needed for full pipelines is present) and public-API discipline (what is actually public). Several architecturally important functions (optimize!, rotation_gridsearch, grid_rotations)
are non-exported despite being documented and tested as user-facing; conversely, fit_sigmoid is exported despite being a narrow internal helper. The two highest-leverage changes are: (1) rationalizing the export list to match actual
user-facing intent, and (2) replacing auto_λ's 6-tuple return with a named result type.

---

Phase 5 — Values Clarification

Before writing the plan, I'd like you to answer these briefly in your own words:

1. Scope and audience: This is clearly the Register.jl ecosystem back-end — but should rigid registration stay here, or is a separate package the right long-term home?
2. Central abstraction: There isn't a single "RegisterOptimize result" type — should there be? Or is the package intentionally a collection of functions rather than a type-centered API?
3. Composability and Base relationship: How aggressively should auto_λ's return value be structured (NamedTuple, struct, keep tuple)? Should AffineQHessian/TimeHessian be public?
4. Error and failure model: Throw vs. return flag (like the isconverged from initial_deformation) vs. silent retry — what should the consistent policy be?
5. Release strategy: Some of the changes above (renaming exports, changing return types) would be breaking. Would you prefer a final 0.3.x non-breaking release before changes land, or batch everything into 0.4.0?