Phase 3 — Coherence Analysis
3a. Scope coherence
The package contains two conceptually distinct algorithmic components:
- Quadratic mismatch fitting + global affine solve: qfit, mms2fit!, qbuild, mismatch2affine — all operate on MismatchArray data from RegisterCore.
- Principal Axes Transform for rigid pre-alignment: principalaxes, pat_rotation — operate on raw image arrays, no mismatch data involved.
optimize_per_aperture is described as "primarily a diagnostic or baseline comparison tool" in its own docstring — it does independent per-aperture argmin
search with no global coupling, which is the opposite of the package's stated purpose.
uisvalid and uclamp! are displacement bounds utilities with domain-specific u prefix names that suggest internal helpers.
3b. Type hierarchy
The package defines no types of its own. The central conceptual unit — the quadratic fit triple (E0, u0, Q) — is returned as a bare unnamed tuple from
qfit. It is then immediately decomposed: mms2fit! stores u0 values in cs, Q values in Qs, and E0 values go into mmis. The triple is never kept together as
a coherent object. mismatch2affine independently re-runs qfit on every aperture, duplicating the iteration logic of mms2fit!.
3c. Overlapping operations
mismatch2affine vs mms2fit!: Both iterate over an array of MismatchArrays and call qfit on each. Neither calls the other. The duplication means that a
user cannot call mms2fit! to pre-compute fits (e.g., for caching or inspection) and then pass those fits to mismatch2affine to perform the global solve.
They must re-run qfit a second time.
qfit(opt=true) vs qfit(opt=false): The opt keyword distinguishes a refined NLsolve path from a fast heuristic. The opt=false path is what mms2fit! always
uses. The opt=true path is presumably what mismatch2affine uses (or the default). These are effectively two algorithms sharing a name and a Bool flag.
optimize_per_aperture vs mismatch2affine: Both produce a per-aperture or global displacement from the same mismatch data. The former does independent
argmin searches; the latter does quadratic fitting and global coupling. These diverge enough in intent that overlap is not a code-quality problem, but the
naming doesn't make the distinction clear.
3d. Implied but absent operations
The pipeline as documented is:
mismatch data → mms2fit! → (cs, Qs, mmis) → ???
mismatch data → mismatch2affine → AffineMap
There is no exported function that accepts the (cs, Qs) output of mms2fit! and assembles the global affine system. A user who calls mms2fit! to inspect
per-aperture fits cannot reuse that work when calling for the global affine — they must call mismatch2affine from scratch.
pat_rotation returns a Vector{AffineMap} of 2–4 candidates (one per 180° ambiguity). No function in the package helps the user select among them — the
selection step is entirely out of scope with no documented escape hatch pointing elsewhere.
mms2fit! has a ! suffix and mutates its mms input (via interpolate_mm! from RegisterPenalty, which prepares arrays for interpolation). The mutation is on
the input data, not the output — a user who later wants to inspect the original mismatch arrays will find them already modified.
3e. Abstraction level
The export list mixes at least three levels:
┌─────────────────────┬─────────────────────────────────┐
│ Level │ Functions │
├─────────────────────┼─────────────────────────────────┤
│ High-level pipeline │ mismatch2affine, pat_rotation │
├─────────────────────┼─────────────────────────────────┤
│ Mid-level steps │ mms2fit!, principalaxes │
├─────────────────────┼─────────────────────────────────┤
│ Low-level utilities │ qfit, qbuild, uisvalid, uclamp! │
└─────────────────────┴─────────────────────────────────┘
uisvalid and uclamp! are particularly low-level: they test and clamp displacement arrays element-wise against per-dimension bounds. They appear to exist
primarily to support internal logic inside qfit and mms2fit!.
3f. Composability
The (E0, u0, Q) tuple returned by qfit does not compose forward into any other exported function. mismatch2affine cannot accept it; qbuild accepts (E0,
umin, Q, maxshift) but that is a reconstruction utility. The mid-level output of mms2fit! (cs, Qs, mmis) cannot be forwarded into mismatch2affine.
The two arms of the package (mismatch fitting vs PAT rotation) do not compose with each other at all — they are independent pipelines that both happen to
return AffineMap. There is no function that chains them.
3g. Relationship to Base and the standard library
uclamp! is semantically clamp! with per-dimension bounds for arrays of scalars or StaticVectors. The name uclamp! gives no indication of how it differs
from Base.clamp!; the u prefix is meaningful only to users who know the domain convention that u means displacement.
principalaxes is a general image-statistics function (intensity-weighted centroid and covariance). It has no dependency on mismatch data and could live in
a standalone statistics utility package. Its presence here is driven by its use inside pat_rotation, but it is general enough that users of other
image-processing packages might independently want it.
3h. Error and failure model
Three distinct behaviors for insufficient data:
- qfit: returns all-zeros (0, zeros, zeros) when no pixels exceed thresh — silent degenerate return.
- mismatch2affine: retries with halved thresh up to 3 times, then falls back to a pure translation — silent recovery with reduced accuracy.
- optimize_per_aperture: unclear from the conceptual map what happens with all-below-threshold apertures.
There is no way for callers to distinguish "fit succeeded" from "fit returned zeros because nothing was valid." The thresh retry logic in mismatch2affine
is also invisible — the caller gets an AffineMap with no indication of how many apertures were actually used or how many times the threshold was relaxed.
3i. Missing public annotations
All non-internal functions appear in docstrings and are tested with qualified calls (RegisterFit.X). Notably, the test suite uses module-qualified names
throughout — even for functions that appear to be exported — which is unusual. This could mean the functions are not actually exported and the "exported"
classification in the conceptual map is incorrect, or it could be a testing convention. If they are not exported, every function in the package is
technically internal.
Phase 4 — Report
Likely design issues
-
No first-class type for the quadratic fit triple.
qfit returns (E0, u0, Q) as a bare tuple. This triple is the central conceptual unit of the package — every downstream operation either consumes it or is
built around it — but it is immediately decomposed everywhere it is used. mms2fit! stores the three components in three separate arrays. mismatch2affine
re-runs qfit independently rather than accepting pre-computed fits. A QuadraticFit (or ApertureFit) struct would give the concept a name, allow
type-stable storage, and open the door to composing mms2fit! output with mismatch2affine's assembly step.
-
mismatch2affine and mms2fit! both re-run qfit on every aperture.
There is no exported path from mms2fit! output → global affine solve. A user who calls mms2fit! to inspect per-aperture fits and then wants a global
affine must re-run the quadratic fitting from scratch inside mismatch2affine. This is both wasteful and forces a choice: use the high-level
mismatch2affine (opaque, no access to intermediate fits) or use mms2fit! (get the fits but lose the global solve). The two levels do not compose.
-
mms2fit! silently mutates its input.
The function name suggests it fills output arrays, but it also calls interpolate_mm! on the input mms argument, permanently modifying the mismatch data. A
caller who inspects mms after mms2fit! will find it transformed. This side effect is not communicated by the function's name or obvious from its
signature.
-
All-zeros return from qfit is indistinguishable from a valid fit at the origin.
When no pixels exceed thresh, qfit returns (0, zeros, zeros) — the same form as a valid fit whose minimum happens to be at the origin with zero curvature.
Callers cannot detect this degenerate case without re-checking the mismatch data themselves.
Design questions
Q1. Does PAT rotation belong in RegisterFit?
principalaxes and pat_rotation operate on raw image arrays and have no dependency on mismatch data. They are a rigid pre-alignment utility that produces
AffineMap candidates via a completely different algorithm. Was the intent to consolidate all "affine registration" functionality in one package? Or did
this code land here for historical reasons and might now warrant its own package (e.g., RegisterPAT) or placement in a more general package?
Q2. Is optimize_per_aperture part of the public interface?
Its docstring describes it as a diagnostic baseline. If it is public, what use case does it serve for end users? If it is primarily for benchmarking the
quadratic fitting approach, should it be in the test suite rather than the package?
Q3. Are uisvalid and uclamp! intended for external use?
Both have domain-specific names (u prefix for displacement) and appear to be used primarily inside the package's own fitting routines. If they are
exported, what external use case motivates that? If they are only needed internally, removing them from the public API would reduce the surface area
without breaking any documented workflow.
Q4. Should there be a function that takes (cs, Qs) and performs the global affine solve?
This would allow the two levels of abstraction to compose: a user could call mms2fit!, inspect the per-aperture fits, filter or reweight them, and then
call fits2affine(cs, Qs, knots) to get the global result. Is the current all-or-nothing design intentional?
Q5. How should the opt parameter in qfit be handled?
opt=true uses NLsolve to refine Q; opt=false uses a fast heuristic. These are meaningfully different algorithms. Is the intent that users should ever set
opt=false directly, or is that purely an internal optimization that should be hidden?
Q6. What happens with below-threshold data in optimize_per_aperture?
The degenerate case handling for qfit is documented (returns zeros). Is there a parallel guarantee for optimize_per_aperture?
Observations
- The thresh retry logic in mismatch2affine (halving up to 3 times) is a policy choice embedded silently in the implementation. Users who want to know
"how many apertures were actually used" or "was the threshold relaxed?" have no way to find out.
- pat_rotation returns 2 or 4 candidate AffineMaps with no ranking or scoring. The selection step (try each candidate, pick the one with best
registration) is entirely the caller's responsibility, with no documented helper or pointer to one.
- The magic constants register_half = 0.5001 and register_half_safe = 0.51 are internal but encode a specific invariant (the boundary between valid and
invalid shift positions). Their values being so close to each other and to 0.5 suggests they encode a convention from the RegisterCore ecosystem — it
would be worth documenting where these numbers come from.
- Tests call all functions with module-qualified names (RegisterFit.qfit, RegisterFit.pat_rotation, etc.). If these are intended to be exported, this is
an unusual testing convention. If they are not exported, then the package's entire public API is accessible only via qualification, which is a significant
usability issue worth confirming either way.
Overall characterization
RegisterFit is a well-focused numerical package with a clear algorithmic core: fit per-aperture mismatch to quadratics, assemble a global linear system,
solve for an affine transform. The implementation is technically sophisticated (generated code for unrolled tensor accumulation, NLsolve-based curvature
refinement). The main design tension is that the package operates at two abstraction levels — a composable, inspectable mid-level (qfit, mms2fit!) and a
high-level black box (mismatch2affine) — but these levels do not compose because there is no named type connecting them and no exported function for the
"global solve from pre-computed fits" step. The highest-leverage changes would be: (1) introduce a QuadraticFit struct so the mid-level output can flow
into the high-level solver, and (2) clarify whether PAT rotation is a permanent resident of this package or a temporary guest.
Phase 5 — Values clarification
Before I write the plan, I'd like you to answer a few questions in your own words. Your answers will serve as the tiebreaker for ambiguous decisions in
the implementation phase.
- Scope and audience: Who is this package for? The conceptual map suggests it's primarily for the HolyLab image registration pipeline — is it intended to
be a general-purpose registration utility that others might adopt, or is it a specialist tool tightly coupled to RegisterCore/RegisterPenalty? And
specifically: is PAT rotation (principalaxes, pat_rotation) a permanent part of RegisterFit's scope, or did it land here for convenience?
- Central abstraction: The (E0, u0, Q) triple is the conceptual core but has no named type. Is the tuple representation intentional (keep it lightweight,
avoid a new type), or would a QuadraticFit struct that lets you compose mms2fit! output with a separate global-solve step be welcome?
- Composability and Base relationship: How important is it that the mid-level (mms2fit!) and high-level (mismatch2affine) pipeline steps compose? Should
a user be able to call mms2fit!, inspect or filter the per-aperture fits, and then feed those into the affine solve without re-running the fitting?
- Error and failure model: Should callers be able to detect degenerate/failed fits? For example, should qfit return a typed sentinel (nothing? a
Missing-like value?) rather than all-zeros, and should mismatch2affine report how many apertures it actually used and whether it relaxed the threshold?
- optimize_per_aperture, uisvalid, uclamp!: Are these intended to be user-facing, or internal implementation details that happened to end up accessible?
Also, since at least one potential change (introducing a named type for the quadratic fit) could be breaking if it changes the return type of qfit:
- Release strategy: Are you open to breaking changes in the near term, or do you need a clean non-breaking release first? Should breaking changes be
batched or released incrementally?
Phase 3 — Coherence Analysis
3a. Scope coherence
The package contains two conceptually distinct algorithmic components:
optimize_per_aperture is described as "primarily a diagnostic or baseline comparison tool" in its own docstring — it does independent per-aperture argmin
search with no global coupling, which is the opposite of the package's stated purpose.
uisvalid and uclamp! are displacement bounds utilities with domain-specific u prefix names that suggest internal helpers.
3b. Type hierarchy
The package defines no types of its own. The central conceptual unit — the quadratic fit triple (E0, u0, Q) — is returned as a bare unnamed tuple from
qfit. It is then immediately decomposed: mms2fit! stores u0 values in cs, Q values in Qs, and E0 values go into mmis. The triple is never kept together as
a coherent object. mismatch2affine independently re-runs qfit on every aperture, duplicating the iteration logic of mms2fit!.
3c. Overlapping operations
mismatch2affine vs mms2fit!: Both iterate over an array of MismatchArrays and call qfit on each. Neither calls the other. The duplication means that a
user cannot call mms2fit! to pre-compute fits (e.g., for caching or inspection) and then pass those fits to mismatch2affine to perform the global solve.
They must re-run qfit a second time.
qfit(opt=true) vs qfit(opt=false): The opt keyword distinguishes a refined NLsolve path from a fast heuristic. The opt=false path is what mms2fit! always
uses. The opt=true path is presumably what mismatch2affine uses (or the default). These are effectively two algorithms sharing a name and a Bool flag.
optimize_per_aperture vs mismatch2affine: Both produce a per-aperture or global displacement from the same mismatch data. The former does independent
argmin searches; the latter does quadratic fitting and global coupling. These diverge enough in intent that overlap is not a code-quality problem, but the
naming doesn't make the distinction clear.
3d. Implied but absent operations
The pipeline as documented is:
mismatch data → mms2fit! → (cs, Qs, mmis) → ???
mismatch data → mismatch2affine → AffineMap
There is no exported function that accepts the (cs, Qs) output of mms2fit! and assembles the global affine system. A user who calls mms2fit! to inspect
per-aperture fits cannot reuse that work when calling for the global affine — they must call mismatch2affine from scratch.
pat_rotation returns a Vector{AffineMap} of 2–4 candidates (one per 180° ambiguity). No function in the package helps the user select among them — the
selection step is entirely out of scope with no documented escape hatch pointing elsewhere.
mms2fit! has a ! suffix and mutates its mms input (via interpolate_mm! from RegisterPenalty, which prepares arrays for interpolation). The mutation is on
the input data, not the output — a user who later wants to inspect the original mismatch arrays will find them already modified.
3e. Abstraction level
The export list mixes at least three levels:
┌─────────────────────┬─────────────────────────────────┐
│ Level │ Functions │
├─────────────────────┼─────────────────────────────────┤
│ High-level pipeline │ mismatch2affine, pat_rotation │
├─────────────────────┼─────────────────────────────────┤
│ Mid-level steps │ mms2fit!, principalaxes │
├─────────────────────┼─────────────────────────────────┤
│ Low-level utilities │ qfit, qbuild, uisvalid, uclamp! │
└─────────────────────┴─────────────────────────────────┘
uisvalid and uclamp! are particularly low-level: they test and clamp displacement arrays element-wise against per-dimension bounds. They appear to exist
primarily to support internal logic inside qfit and mms2fit!.
3f. Composability
The (E0, u0, Q) tuple returned by qfit does not compose forward into any other exported function. mismatch2affine cannot accept it; qbuild accepts (E0,
umin, Q, maxshift) but that is a reconstruction utility. The mid-level output of mms2fit! (cs, Qs, mmis) cannot be forwarded into mismatch2affine.
The two arms of the package (mismatch fitting vs PAT rotation) do not compose with each other at all — they are independent pipelines that both happen to
return AffineMap. There is no function that chains them.
3g. Relationship to Base and the standard library
uclamp! is semantically clamp! with per-dimension bounds for arrays of scalars or StaticVectors. The name uclamp! gives no indication of how it differs
from Base.clamp!; the u prefix is meaningful only to users who know the domain convention that u means displacement.
principalaxes is a general image-statistics function (intensity-weighted centroid and covariance). It has no dependency on mismatch data and could live in
a standalone statistics utility package. Its presence here is driven by its use inside pat_rotation, but it is general enough that users of other
image-processing packages might independently want it.
3h. Error and failure model
Three distinct behaviors for insufficient data:
There is no way for callers to distinguish "fit succeeded" from "fit returned zeros because nothing was valid." The thresh retry logic in mismatch2affine
is also invisible — the caller gets an AffineMap with no indication of how many apertures were actually used or how many times the threshold was relaxed.
3i. Missing public annotations
All non-internal functions appear in docstrings and are tested with qualified calls (RegisterFit.X). Notably, the test suite uses module-qualified names
throughout — even for functions that appear to be exported — which is unusual. This could mean the functions are not actually exported and the "exported"
classification in the conceptual map is incorrect, or it could be a testing convention. If they are not exported, every function in the package is
technically internal.
Phase 4 — Report
Likely design issues
No first-class type for the quadratic fit triple.
qfit returns (E0, u0, Q) as a bare tuple. This triple is the central conceptual unit of the package — every downstream operation either consumes it or is
built around it — but it is immediately decomposed everywhere it is used. mms2fit! stores the three components in three separate arrays. mismatch2affine
re-runs qfit independently rather than accepting pre-computed fits. A QuadraticFit (or ApertureFit) struct would give the concept a name, allow
type-stable storage, and open the door to composing mms2fit! output with mismatch2affine's assembly step.
mismatch2affine and mms2fit! both re-run qfit on every aperture.
There is no exported path from mms2fit! output → global affine solve. A user who calls mms2fit! to inspect per-aperture fits and then wants a global
affine must re-run the quadratic fitting from scratch inside mismatch2affine. This is both wasteful and forces a choice: use the high-level
mismatch2affine (opaque, no access to intermediate fits) or use mms2fit! (get the fits but lose the global solve). The two levels do not compose.
mms2fit! silently mutates its input.
The function name suggests it fills output arrays, but it also calls interpolate_mm! on the input mms argument, permanently modifying the mismatch data. A
caller who inspects mms after mms2fit! will find it transformed. This side effect is not communicated by the function's name or obvious from its
signature.
All-zeros return from qfit is indistinguishable from a valid fit at the origin.
When no pixels exceed thresh, qfit returns (0, zeros, zeros) — the same form as a valid fit whose minimum happens to be at the origin with zero curvature.
Callers cannot detect this degenerate case without re-checking the mismatch data themselves.
Design questions
Q1. Does PAT rotation belong in RegisterFit?
principalaxes and pat_rotation operate on raw image arrays and have no dependency on mismatch data. They are a rigid pre-alignment utility that produces
AffineMap candidates via a completely different algorithm. Was the intent to consolidate all "affine registration" functionality in one package? Or did
this code land here for historical reasons and might now warrant its own package (e.g., RegisterPAT) or placement in a more general package?
Q2. Is optimize_per_aperture part of the public interface?
Its docstring describes it as a diagnostic baseline. If it is public, what use case does it serve for end users? If it is primarily for benchmarking the
quadratic fitting approach, should it be in the test suite rather than the package?
Q3. Are uisvalid and uclamp! intended for external use?
Both have domain-specific names (u prefix for displacement) and appear to be used primarily inside the package's own fitting routines. If they are
exported, what external use case motivates that? If they are only needed internally, removing them from the public API would reduce the surface area
without breaking any documented workflow.
Q4. Should there be a function that takes (cs, Qs) and performs the global affine solve?
This would allow the two levels of abstraction to compose: a user could call mms2fit!, inspect the per-aperture fits, filter or reweight them, and then
call fits2affine(cs, Qs, knots) to get the global result. Is the current all-or-nothing design intentional?
Q5. How should the opt parameter in qfit be handled?
opt=true uses NLsolve to refine Q; opt=false uses a fast heuristic. These are meaningfully different algorithms. Is the intent that users should ever set
opt=false directly, or is that purely an internal optimization that should be hidden?
Q6. What happens with below-threshold data in optimize_per_aperture?
The degenerate case handling for qfit is documented (returns zeros). Is there a parallel guarantee for optimize_per_aperture?
Observations
"how many apertures were actually used" or "was the threshold relaxed?" have no way to find out.
registration) is entirely the caller's responsibility, with no documented helper or pointer to one.
invalid shift positions). Their values being so close to each other and to 0.5 suggests they encode a convention from the RegisterCore ecosystem — it
would be worth documenting where these numbers come from.
an unusual testing convention. If they are not exported, then the package's entire public API is accessible only via qualification, which is a significant
usability issue worth confirming either way.
Overall characterization
RegisterFit is a well-focused numerical package with a clear algorithmic core: fit per-aperture mismatch to quadratics, assemble a global linear system,
solve for an affine transform. The implementation is technically sophisticated (generated code for unrolled tensor accumulation, NLsolve-based curvature
refinement). The main design tension is that the package operates at two abstraction levels — a composable, inspectable mid-level (qfit, mms2fit!) and a
high-level black box (mismatch2affine) — but these levels do not compose because there is no named type connecting them and no exported function for the
"global solve from pre-computed fits" step. The highest-leverage changes would be: (1) introduce a QuadraticFit struct so the mid-level output can flow
into the high-level solver, and (2) clarify whether PAT rotation is a permanent resident of this package or a temporary guest.
Phase 5 — Values clarification
Before I write the plan, I'd like you to answer a few questions in your own words. Your answers will serve as the tiebreaker for ambiguous decisions in
the implementation phase.
be a general-purpose registration utility that others might adopt, or is it a specialist tool tightly coupled to RegisterCore/RegisterPenalty? And
specifically: is PAT rotation (principalaxes, pat_rotation) a permanent part of RegisterFit's scope, or did it land here for convenience?
avoid a new type), or would a QuadraticFit struct that lets you compose mms2fit! output with a separate global-solve step be welcome?
a user be able to call mms2fit!, inspect or filter the per-aperture fits, and then feed those into the affine solve without re-running the fitting?
Missing-like value?) rather than all-zeros, and should mismatch2affine report how many apertures it actually used and whether it relaxed the threshold?
Also, since at least one potential change (introducing a named type for the quadratic fit) could be breaking if it changes the return type of qfit:
batched or released incrementally?