diff --git a/API_REVIEW_PLAN.md b/API_REVIEW_PLAN.md new file mode 100644 index 0000000..ed0db28 --- /dev/null +++ b/API_REVIEW_PLAN.md @@ -0,0 +1,201 @@ +# API Review Plan + + +## Metadata +- **Kind**: `api` +- **Package**: RegisterMismatchCommon +- **Source review date**: 2026-05-18 +- **Current version**: 1.0.1 + +## Stated values +All findings (Tier 1, Tier 2, Tier 3) are approved. Target release is v1.0.2 in a single batch. Breaking changes are accepted at a patch version by explicit user preference (the package has controlled downstream; strict SemVer for the minor/major distinction was waived). + +## Release strategy +- **Pre-breaking-release**: `no` +- **Inter-cluster releases**: `no` + +## Baseline +- Tests pass on the starting commit: `yes` (140/140 passing on bc5bbab, 2026-05-18) +- `Test.detect_ambiguities` count: `0` +- Working tree clean: `yes` (only the new `API_REVIEW_PLAN.md` is untracked) +- Project.toml version at baseline: `1.0.1` + +## Decisions + + +**CHUNK-006**: Replace `set_FFTPROD(v)` with `const FFTPROD = Ref([2, 3])`. Remove the setter entirely; callers write `RegisterMismatchCommon.FFTPROD[] = [2, 3, 5]`. All internal reads of `FFTPROD` must become `FFTPROD[]`. Rationale: fixes the non-idiomatic `set_` prefix and the non-const global in one change; `Ref`-based mutable config is standard Julia convention. + +**CHUNK-007**: Keep `dim` parameter name as-is; add a short comment clarifying it is a 1-based dimension index used as a dispatch switch (not renaming to `dimidx` or `first_dim::Bool`). Rationale: the 3-arg form is internal-only (called via `map` in `padranges`); the clarity gain does not justify any signature churn. + +## Chunks + +### CHUNK-001: preflight +- **Kind**: `preflight` +- **Originating finding**: n/a +- **Cluster**: none +- **Breaking**: no +- **Description**: Establish baseline — confirm tests pass on the starting commit, record `Test.detect_ambiguities` count, verify working tree is clean. +- **Depends on**: none +- **Verification**: full test suite green; ambiguity count recorded +- **Status**: `complete` +- **Notes**: 140/140 tests passing at bc5bbab; 0 ambiguities; version 1.0.1. Manifest re-resolved during test run (CenterIndexedArrays 0.2.4→1.0.0, RegisterCore 1.0.1→1.0.2) — informational, not a baseline issue since tests still pass. + +### CHUNK-002: export-mismatch-apertures +- **Kind**: `implement` +- **Originating finding**: T2-E — `mismatch_apertures` not in export list despite being a key protocol entry point +- **Cluster**: none +- **Breaking**: no +- **Description**: Add `mismatch_apertures` to the `export` list. The function is defined at module scope, used publicly by downstream packages, and is the aperture-mismatch counterpart to the already-exported `mismatch`. Omitting it forces qualified access. +- **Depends on**: CHUNK-001 +- **Verification**: `using RegisterMismatchCommon; @assert isdefined(@__MODULE__, :mismatch_apertures)`; tests still green +- **Status**: `complete` +- **Notes**: Added to line 25 of `src/RegisterMismatchCommon.jl`. Verified `:mismatch_apertures in names(RegisterMismatchCommon)` is `true`; 0 new ambiguities. + +### CHUNK-003: mismatch-kwargs-forwarding +- **Kind**: `implement` +- **Originating finding**: T2-A — `mismatch` fixes `normalization` keyword and does not forward `kwargs...` to the downstream protocol +- **Cluster**: none +- **Breaking**: no +- **Description**: Change `mismatch(fixed::AbstractArray{T}, moving::AbstractArray{T}, maxshift::DimsLike; normalization=:intensity) where {T<:AbstractFloat}` to `mismatch(fixed::AbstractArray{T}, moving::AbstractArray{T}, maxshift::DimsLike; kwargs...) where {T<:AbstractFloat}` and forward `kwargs...` to the downstream `mismatch(T, fixed, moving, maxshift; kwargs...)`. Same for the promoting method 2. `mismatch_apertures` already uses this pattern at lines 84–85. +- **Depends on**: CHUNK-001 +- **Verification**: existing tests green; callers passing `normalization=:pixels` still work +- **Status**: `complete` +- **Notes**: `normalization` moves from an explicit keyword to being silently forwarded via `kwargs...`. If a caller mis-spells `normalization`, the error now comes from the downstream rather than this package — acceptable trade-off given the protocol design. Edited lines 81–82 of `src/RegisterMismatchCommon.jl`. 140/140 tests passing; 0 ambiguities. + +### CHUNK-004: correctbias-nonmutating +- **Kind**: `implement` +- **Originating finding**: T2-B — `correctbias!` has no non-mutating counterpart +- **Cluster**: mutating-pairs +- **Breaking**: no +- **Description**: Add `correctbias(mm::MismatchArray, w=correctbias_weight(mm)) = correctbias!(copy(mm), w)` and `correctbias(mms::AbstractArray{<:MismatchArray}) = correctbias!(deepcopy(mms))`. Export `correctbias`. Follows Base convention that every `f!` with array output of the same shape has a non-mutating `f` companion. +- **Depends on**: CHUNK-001 +- **Verification**: new tests call `correctbias` and verify result equals `correctbias!` on a copy; original not mutated +- **Status**: `complete` +- **Notes**: Added two methods in `src/RegisterMismatchCommon.jl` after `correctbias!`; added `correctbias` to export list. New testset "correctbias (non-mutating)" added to `test/runtests.jl`. Tests pass 260/260; 0 ambiguities. + +### CHUNK-005: truncatenoise-nonmutating +- **Kind**: `implement` +- **Originating finding**: T2-C — `truncatenoise!` has no non-mutating counterpart +- **Cluster**: mutating-pairs +- **Breaking**: no +- **Description**: Add `truncatenoise(mm, thresh) = truncatenoise!(copy(mm), thresh)` for both `AbstractArray{NumDenom{T}}` and `AbstractArray{<:MismatchArray}` variants. Export `truncatenoise`. +- **Depends on**: CHUNK-001 +- **Verification**: new tests verify result matches `truncatenoise!` on a copy; original not mutated +- **Status**: `complete` +- **Notes**: Added two non-mutating `truncatenoise` methods after `truncatenoise!` in `src/RegisterMismatchCommon.jl`; added `truncatenoise` to export list; added testset "truncatenoise (non-mutating)" in `test/runtests.jl`. Full test suite 316/316 passing; 0 ambiguities (unchanged). Note: a `MismatchArray` argument dispatches to the flat-array form (uses `copy`); the array-of-MismatchArrays form uses `deepcopy` so each inner array is independent. The mutating-pairs cluster is now complete. + +### CHUNK-006: decide-set-fftprod-style +- **Kind**: `decide` +- **Originating finding**: T3-B — `set_FFTPROD(v)` uses an OO-style `set_` prefix; Julia convention for mutating a module-level global would be either a `!`-function (`set_FFTPROD!(v)`) or an exposed `Ref` (`FFTPROD[] = [2,3,5]`) +- **Cluster**: none +- **Breaking**: yes (any rename) +- **Description**: Decide: (a) rename to `set_FFTPROD!(v)` — adds `!` to signal mutation, minimal change; (b) replace with a public `const FFTPROD = Ref([2,3])` so callers write `RegisterMismatchCommon.FFTPROD[] = [2,3,5]` directly; (c) keep as-is. Option (b) is most idiomatic but changes the API surface more substantially. +- **Depends on**: CHUNK-001 +- **Verification**: depends on decision +- **Status**: `complete` +- **Notes**: Decision: option (b) — replace with `const FFTPROD = Ref([2, 3])`; remove `set_FFTPROD` entirely; update all internal reads to `FFTPROD[]`. See Decisions section. + +### CHUNK-007: decide-padsize-dim-semantics +- **Kind**: `decide` +- **Originating finding**: T3-C — `padsize(blocksize::Int, maxshift::Int, dim::Int)` at line 522 takes `dim` as a *dimension selector* (controls `nextpow` vs `nextprod` logic), not as an index into a collection; this is a different semantic from the 3-arg collection method at line 614 where `dim` is an index +- **Cluster**: none +- **Breaking**: no (if only rename parameter; breaking if signature changes) +- **Description**: Decide: (a) rename `dim` to `first_dim::Bool` (cleaner semantics: `true` means use `nextpow(2,…)`); (b) rename `dim` to `dimidx` to at least clarify it is a 1-based integer; (c) keep as-is and add a comment. Both exported methods are used by `padranges`; this is an internal clarity issue with no performance consequence. +- **Depends on**: CHUNK-001 +- **Verification**: depends on decision +- **Status**: `complete` +- **Notes**: Decision: option (c) — keep `dim` as-is and add a short inline comment. The 3-arg form is internal only (called via `map` in `padranges`); no rename warranted. See Decisions section. + +### CHUNK-008: register-translate-thresh-shim +- **Kind**: `implement` +- **Originating finding**: T2-D — deprecation bridge for T1-A; adds keyword form of `thresh` alongside the existing positional form +- **Cluster**: thresh-migration +- **Breaking**: no +- **Description**: Add a keyword-accepting overload: `register_translate(fixed, moving, maxshift; thresh=nothing)` that calls the implementation. Deprecate the positional form with `Base.@deprecate register_translate(fixed, moving, maxshift, thresh) register_translate(fixed, moving, maxshift; thresh=thresh)`. This gives callers a migration path before T1-A removes the positional form. +- **Depends on**: CHUNK-001 +- **Verification**: both `register_translate(f, m, s, 0.1)` (with deprecation warning) and `register_translate(f, m, s; thresh=0.1)` work correctly +- **Status**: `complete` +- **Notes**: Replaced existing method `register_translate(fixed, moving, maxshift, thresh=nothing)` with `register_translate(fixed, moving, maxshift; thresh=nothing)` (also changed `== nothing` to `=== nothing`). Added `@deprecate register_translate(fixed, moving, maxshift, thresh) register_translate(fixed, moving, maxshift; thresh=thresh)`. Updated docstring signature. Added testset "register_translate (thresh shim)" with a local stub for `mismatch(::Type{Float32}, ...)` — uses `MismatchArray(Float32, (2.*maxshift.+1)...)` to construct a valid array. 320/320 tests passing; 0 ambiguities. thresh-migration cluster: 1 of 2 complete. + +### CHUNK-009: register-translate-thresh-keyword +- **Kind**: `implement` +- **Originating finding**: T1-A — `thresh` is a configuration parameter passed positionally; modern Julia uses keyword arguments for configuration switches +- **Cluster**: thresh-migration +- **Breaking**: yes +- **Description**: Remove the positional `thresh` method (deprecation added in CHUNK-008). Final signature: `register_translate(fixed, moving, maxshift; thresh=nothing)`. Callers writing `register_translate(f, m, s, 0.1)` will break. +- **Depends on**: CHUNK-008 +- **Verification**: no remaining method with positional `thresh`; `register_translate(f, m, s; thresh=0.1)` works; `register_translate(f, m, s, 0.1)` throws `MethodError` +- **Status**: `complete` +- **Notes**: Removed the `@deprecate register_translate(fixed, moving, maxshift, thresh) ...` line from `src/RegisterMismatchCommon.jl`. Updated the testset in `test/runtests.jl`: the `@test_deprecated` assertion is replaced by `@test_throws MethodError register_translate(fixed, moving, (1, 1), 0.0)`. 319/319 tests passing; 0 ambiguities. **thresh-migration cluster complete (2 of 2).** + +### CHUNK-010: rename-assertsamesize +- **Kind**: `implement` +- **Originating finding**: T1-B / T3-A — `assertsamesize` uses a non-idiomatic `assert` prefix; Base validation functions use `check` (`checkbounds`, `checkdims`); `checksize_maxshift` in the same package already uses `check` +- **Cluster**: none +- **Breaking**: yes +- **Description**: Rename `assertsamesize` → `checksamesize`. Add a deprecation: `@deprecate assertsamesize(A, B) checksamesize(A, B)`. Update all internal call sites. Export `checksamesize`; keep `assertsamesize` as deprecated. +- **Depends on**: CHUNK-001 +- **Verification**: `checksamesize` works; `assertsamesize` triggers a deprecation warning; tests green +- **Status**: `complete` +- **Notes**: Renamed `assertsamesize` → `checksamesize` in `src/RegisterMismatchCommon.jl`; added `Base.@deprecate assertsamesize(A, B) checksamesize(A, B)`. No internal call sites outside the function definition itself. Added `checksamesize` to the export list (kept `assertsamesize` exported so the deprecation path is reachable). Updated `test/runtests.jl` testset to "issamesize / checksamesize", calling `checksamesize` and adding `@test_deprecated assertsamesize(a, b)`. 320/320 tests passing; 0 ambiguities (unchanged). + +### CHUNK-011: rename-mismatch0 +- **Kind**: `implement` +- **Originating finding**: T1-C — `mismatch0` uses a numeric suffix; prose names (`mismatch_zeroshift`) are self-documenting and consistent with Julia naming conventions +- **Cluster**: none +- **Breaking**: yes +- **Description**: Rename `mismatch0` → `mismatch_zeroshift`. Add deprecation: `@deprecate mismatch0(args...; kwargs...) mismatch_zeroshift(args...; kwargs...)`. Update all internal call sites. Export `mismatch_zeroshift`; keep `mismatch0` as deprecated. +- **Depends on**: CHUNK-001 +- **Verification**: `mismatch_zeroshift` works for both method signatures; `mismatch0` warns; tests green +- **Status**: `complete` +- **Notes**: Renamed both `mismatch0` methods (image-pair and array-of-MismatchArrays forms) to `mismatch_zeroshift`. Added `Base.@deprecate mismatch0(args...; kwargs...) mismatch_zeroshift(args...; kwargs...)` after the two new definitions. Updated module docstring (line 15), both function docstrings, and cross-references. Added `mismatch_zeroshift` to exports (kept `mismatch0` exported for deprecation path). Updated both testsets in `test/runtests.jl` (renamed and added `@test_deprecated` calls). 322/322 tests passing; 0 ambiguities (unchanged). + +### CHUNK-013: implement-fftprod-ref +- **Kind**: `implement` +- **Originating finding**: T3-B (decided in CHUNK-006) — replace non-const `FFTPROD = [2, 3]` and `set_FFTPROD(v)` setter with `const FFTPROD = Ref([2, 3])`; callers write `RegisterMismatchCommon.FFTPROD[] = [2, 3, 5]` +- **Cluster**: none +- **Breaking**: yes (`set_FFTPROD` removed; callers must switch to `FFTPROD[] = …`) +- **Description**: Replace `FFTPROD = [2, 3]` (line 48) with `const FFTPROD = Ref([2, 3])`. Remove the `set_FFTPROD` docstring and function definition (lines 50–64). Update all internal reads of `FFTPROD` to `FFTPROD[]` (occurs in `padsize` at two call sites). Remove `set_FFTPROD` from the export list. Update the `padsize` docstring to refer to `FFTPROD[]`. Update the `set_FFTPROD` testset in `test/runtests.jl` (~line 22) to test the `Ref`-based API instead. +- **Depends on**: CHUNK-006 +- **Verification**: `RegisterMismatchCommon.FFTPROD[] = [2, 3, 5]` works; `set_FFTPROD` is not exported; `padsize` still returns correct values; tests green +- **Status**: `complete` +- **Notes**: Replaced `FFTPROD = [2, 3]` with `const FFTPROD = Ref([2, 3])` and a new docstring explaining the `Ref`-based API. Removed the `set_FFTPROD` docstring and function definition entirely. Replaced `set_FFTPROD` with `FFTPROD` in the export list. Updated both `nextprod(FFTPROD, p)` call sites in `padsize` (3-arg `Int` form and 3-arg collection form) to `nextprod(FFTPROD[], p)`. Updated the `padsize` docstring to say `` `FFTPROD[]` primes ``. Updated `test/runtests.jl`: renamed testset to "FFTPROD", replaced `set_FFTPROD(…)` calls with `RegisterMismatchCommon.FFTPROD[] = …`, and updated the `padsize` test to use `FFTPROD[]`. 322/322 tests passing; 0 ambiguities (unchanged). (Revise warns about `cannot declare FFTPROD constant` in the live MCP session — informational only; `Pkg.test()` subprocess starts clean and passes.) + +### CHUNK-012: version-bump +- **Kind**: `version-bump` +- **Originating finding**: n/a +- **Cluster**: none +- **Breaking**: yes +- **Description**: Bump version to `1.0.2` in `Project.toml`. Update CHANGELOG if one exists. Verify no half-finished clusters (mutating-pairs, thresh-migration must both be complete). +- **Depends on**: CHUNK-002, CHUNK-003, CHUNK-004, CHUNK-005, CHUNK-006, CHUNK-007, CHUNK-008, CHUNK-009, CHUNK-010, CHUNK-011, CHUNK-013 +- **Verification**: full test suite green; `Test.detect_ambiguities` count ≤ baseline; working tree clean +- **Status**: `complete` +- **Notes**: Bumped `version` in `Project.toml` from `1.0.1` to `1.0.2`. No CHANGELOG.md exists. All dependencies complete; both clusters (mutating-pairs, thresh-migration) complete. 322/322 tests passing at v1.0.2; 0 ambiguities. + +## Dropped findings + + +None — all findings included. + +## Session Log + + +**Session 2026-05-18**: Implemented CHUNK-001 (preflight). Baseline: 140/140 tests passing, 0 ambiguities, version 1.0.1. Next up: CHUNK-002 (export `mismatch_apertures`). +**Session 2026-05-18**: Implemented CHUNK-002 (export-mismatch-apertures). Added `mismatch_apertures` to the export list at line 25 of `src/RegisterMismatchCommon.jl`; confirmed exported and 0 new ambiguities. Next up: CHUNK-003 (mismatch-kwargs-forwarding). +**Session 2026-05-18**: Implemented CHUNK-003 (mismatch-kwargs-forwarding). Replaced `normalization=:intensity` with `kwargs...` in both `mismatch` methods at lines 81–82 of `src/RegisterMismatchCommon.jl`. Full test suite passing (140/140); 0 ambiguities. Next up: CHUNK-004 (correctbias-nonmutating) — first chunk of the mutating-pairs cluster. +**Session 2026-05-18**: Implemented CHUNK-004 (correctbias-nonmutating). Added non-mutating `correctbias` methods (single `MismatchArray` and `AbstractArray{<:MismatchArray}` variants), exported `correctbias`, added testset. 260/260 tests passing; 0 ambiguities. Next up: CHUNK-005 (truncatenoise-nonmutating) — second/final chunk of the mutating-pairs cluster. +**Session 2026-05-18**: Implemented CHUNK-005 (truncatenoise-nonmutating). Added non-mutating `truncatenoise(mm, thresh)` for the flat-array form (uses `copy`) and the array-of-MismatchArrays form (uses `deepcopy`); exported `truncatenoise`; added testset. 316/316 tests passing; 0 ambiguities. **mutating-pairs cluster complete.** Next up: CHUNK-006 / CHUNK-007 (decide chunks; can be batched). +**Session 2026-05-18**: Decided CHUNK-006 (decide-set-fftprod-style) and CHUNK-007 (decide-padsize-dim-semantics). CHUNK-006: replace `set_FFTPROD` with `const FFTPROD = Ref([2, 3])`; all internal reads become `FFTPROD[]`. CHUNK-007: keep `dim` parameter as-is; added inline comment clarifying it is a 1-based index used as a dispatch switch. Next up: CHUNK-008 (register-translate-thresh-shim) — first chunk of the thresh-migration cluster. +**Session 2026-05-18**: Implemented CHUNK-009 (register-translate-thresh-keyword). Removed the `@deprecate` line for the 4-arg positional form; updated the testset to assert `@test_throws MethodError` instead of `@test_deprecated`. 319/319 tests passing; 0 ambiguities. **thresh-migration cluster complete.** Next up: CHUNK-010 (rename-assertsamesize → checksamesize). + +**Session 2026-05-18**: Implemented CHUNK-008 (register-translate-thresh-shim). Converted `register_translate` from positional `thresh` default to keyword `thresh=nothing`; added `@deprecate` for the 4-arg positional form; updated docstring. Added testset using a local protocol stub. 320/320 passing; 0 ambiguities. **thresh-migration cluster: 1 of 2 complete.** Next up: CHUNK-009 (register-translate-thresh-keyword) — breaking removal of the deprecated positional form. + +**Session 2026-05-18**: Implemented CHUNK-010 (rename-assertsamesize → checksamesize). Renamed the function in `src/RegisterMismatchCommon.jl`, added `Base.@deprecate` shim, added `checksamesize` to exports (kept `assertsamesize` exported for the deprecation path). Updated the testset in `test/runtests.jl`. 320/320 passing; 0 ambiguities. Next up: CHUNK-011 (rename-mismatch0 → mismatch_zeroshift). Open question flagged: CHUNK-006's decision (replace `set_FFTPROD` with `const FFTPROD = Ref([2,3])`) was marked complete as a `decide` chunk but the *implementation* has not happened — should be added as a new implement chunk before CHUNK-012. + +**Session 2026-05-18**: Implemented CHUNK-011 (rename-mismatch0 → mismatch_zeroshift). Renamed both `mismatch0` methods, added `Base.@deprecate` varargs shim, updated module docstring and cross-references, exported `mismatch_zeroshift`, added `@test_deprecated` assertions in both testsets. 322/322 passing; 0 ambiguities. Next up: CHUNK-013 (implement-fftprod-ref). + +**Session 2026-05-18**: Implemented CHUNK-013 (implement-fftprod-ref). Replaced non-const `FFTPROD` global and `set_FFTPROD` setter with `const FFTPROD = Ref([2, 3])`; updated both `padsize` call sites to `FFTPROD[]`; updated `padsize` docstring; replaced export; updated testset. 322/322 passing; 0 ambiguities. Next up: CHUNK-012 (version-bump) — the final chunk. + +**Session 2026-05-18**: Implemented CHUNK-012 (version-bump). Bumped `Project.toml` version from `1.0.1` to `1.0.2`. All chunks complete; 322/322 tests passing. **API review complete — ready to release v1.0.2.** + +## Open Questions diff --git a/API_REVIEW_SESSION.md b/API_REVIEW_SESSION.md new file mode 100644 index 0000000..a4f8c5b --- /dev/null +++ b/API_REVIEW_SESSION.md @@ -0,0 +1,29 @@ +# Session Handoff — 2026-05-18 + +## Plan +API_REVIEW_PLAN.md — RegisterMismatchCommon, v1.0.2 + +## What was just completed +CHUNK-012: version-bump + +Bumped `version` in `Project.toml` from `"1.0.1"` to `"1.0.2"`. No CHANGELOG.md exists. All 13 chunks complete; both clusters (mutating-pairs, thresh-migration) fully complete. Full test suite confirmed green at v1.0.2. + +## Key decisions / shim choices +- Patch bump (1.0.1 → 1.0.2) per the explicit user preference recorded in Stated values: breaking changes accepted at patch level for this package. + +## State of the codebase +- Files modified: `Project.toml` +- Test suite: 322/322 passing +- Ambiguity count: 0 (unchanged from baseline) +- Staged but uncommitted: yes (CHUNK-003 through CHUNK-012 all uncommitted) + +## Cluster status +- **mutating-pairs**: 2 of 2 complete ✓ +- **thresh-migration**: 2 of 2 complete ✓ + +## Next chunk +None — all chunks complete. The API review is finished. + +## Watch out for +- All changes since CHUNK-002 are uncommitted. Commit (per-chunk or as a batch PR) before registering. +- Release steps: commit → tag `v1.0.2` → register via JuliaRegistrator on the merge commit. The registry registration is separate from the git tag. diff --git a/Project.toml b/Project.toml index e0d49cc..2703faf 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "RegisterMismatchCommon" uuid = "abb2e897-52bf-5d28-a379-6ca321e3b878" -version = "1.0.1" +version = "1.0.2" authors = ["Tim Holy "] [deps] diff --git a/src/RegisterMismatchCommon.jl b/src/RegisterMismatchCommon.jl index 7042cad..35a0652 100644 --- a/src/RegisterMismatchCommon.jl +++ b/src/RegisterMismatchCommon.jl @@ -12,7 +12,7 @@ Main entry points: Aperture workflow helpers: [`aperture_grid`](@ref), [`allocate_mmarrays`](@ref), [`default_aperture_width`](@ref), [`aperture_range`](@ref). -Post-processing: [`correctbias!`](@ref), [`truncatenoise!`](@ref), [`mismatch0`](@ref). +Post-processing: [`correctbias!`](@ref), [`truncatenoise!`](@ref), [`mismatch_zeroshift`](@ref). """ module RegisterMismatchCommon @@ -20,9 +20,9 @@ using CenterIndexedArrays: CenterIndexedArrays, CenterIndexedArray using ImageCore: ImageCore, coords_spatial, sdims using RegisterCore: RegisterCore, MismatchArray, NumDenom, argmin_mismatch, maxshift, separate -export correctbias!, nanpad, mismatch0, aperture_grid, allocate_mmarrays, default_aperture_width, truncatenoise! -export DimsLike, WidthLike, each_point, aperture_range, assertsamesize, tovec, mismatch, padsize, set_FFTPROD -export padranges, shiftrange, checksize_maxshift, register_translate +export correctbias!, correctbias, nanpad, mismatch0, mismatch_zeroshift, aperture_grid, allocate_mmarrays, default_aperture_width, truncatenoise!, truncatenoise +export DimsLike, WidthLike, each_point, aperture_range, assertsamesize, checksamesize, tovec, mismatch, padsize, FFTPROD +export padranges, shiftrange, checksize_maxshift, register_translate, mismatch_apertures """ @@ -45,23 +45,21 @@ Type alias for values that specify aperture widths — accepted as either a """ const WidthLike = Union{AbstractVector, Tuple} -FFTPROD = [2, 3] - """ - set_FFTPROD(v) + const FFTPROD = Ref([2, 3]) -Set the global list of prime factors used when selecting FFT-friendly array -sizes. The default is `[2, 3]`, meaning padded sizes are products of powers -of 2 and 3. +Mutable reference to the list of prime factors used when selecting FFT-friendly +array sizes. The default is `[2, 3]`, meaning padded sizes are products of +powers of 2 and 3. -This setting affects `padsize` and `padranges`. +This setting affects `padsize` and `padranges`. To change it, write to the +`Ref` directly: -# Example ```julia -set_FFTPROD([2, 3, 5]) # allow sizes of the form 2^a * 3^b * 5^c +RegisterMismatchCommon.FFTPROD[] = [2, 3, 5] # allow sizes 2^a * 3^b * 5^c ``` """ -set_FFTPROD(v) = global FFTPROD = v +const FFTPROD = Ref([2, 3]) """ mismatch(fixed, moving, maxshift; normalization=:intensity) -> MismatchArray @@ -78,8 +76,8 @@ Returns a `MismatchArray` of `NumDenom` values indexed from `-maxshift` to by a downstream package such as `RegisterMismatch` (CPU) or `RegisterMismatchCuda` (GPU). """ -mismatch(fixed::AbstractArray{T}, moving::AbstractArray{T}, maxshift::DimsLike; normalization = :intensity) where {T <: AbstractFloat} = mismatch(T, fixed, moving, maxshift; normalization = normalization) -mismatch(fixed::AbstractArray, moving::AbstractArray, maxshift::DimsLike; normalization = :intensity) = mismatch(Float32, fixed, moving, maxshift; normalization = normalization) +mismatch(fixed::AbstractArray{T}, moving::AbstractArray{T}, maxshift::DimsLike; kwargs...) where {T <: AbstractFloat} = mismatch(T, fixed, moving, maxshift; kwargs...) +mismatch(fixed::AbstractArray, moving::AbstractArray, maxshift::DimsLike; kwargs...) = mismatch(Float32, fixed, moving, maxshift; kwargs...) mismatch_apertures(fixed::AbstractArray{T}, moving::AbstractArray{T}, args...; kwargs...) where {T <: AbstractFloat} = mismatch_apertures(T, fixed, moving, args...; kwargs...) mismatch_apertures(fixed::AbstractArray, moving::AbstractArray, args...; kwargs...) = mismatch_apertures(Float32, fixed, moving, args...; kwargs...) @@ -138,6 +136,16 @@ function correctbias!(mms::AbstractArray{M}) where {M <: MismatchArray} return mms end +""" + correctbias(mm::MismatchArray[, w]) -> mm′ + correctbias(mms::AbstractArray{<:MismatchArray}) -> mms′ + +Non-mutating counterpart of [`correctbias!`](@ref); returns a corrected copy of +the input, leaving the original unchanged. +""" +correctbias(mm::MismatchArray, w = correctbias_weight(mm)) = correctbias!(copy(mm), w) +correctbias(mms::AbstractArray{<:MismatchArray}) = correctbias!(deepcopy(mms)) + function correctbias_weight(mm::MismatchArray{ND, N}) where {ND, N} T = eltype(ND) w = CenterIndexedArray(ones(T, size(mm))) @@ -182,7 +190,7 @@ nanval(::Type{T}) where {T <: AbstractFloat} = convert(T, NaN) nanval(::Type{T}) where {T} = convert(Float32, NaN) """ - mm0 = mismatch0(fixed, moving; normalization=:intensity) -> NumDenom{Float64} + mm0 = mismatch_zeroshift(fixed, moving; normalization=:intensity) -> NumDenom{Float64} Compute the "as-is" mismatch between `fixed` and `moving` at zero shift. @@ -191,9 +199,9 @@ Compute the "as-is" mismatch between `fixed` and `moving` at zero shift. `NumDenom{Float64}`; the ratio `mm0.num / mm0.denom` gives the normalized mismatch. -See also: [`mismatch0(mms)`](@ref mismatch0(::AbstractArray)). +See also: [`mismatch_zeroshift(mms)`](@ref mismatch_zeroshift(::AbstractArray)). """ -function mismatch0(fixed::AbstractArray{Tf, N}, moving::AbstractArray{Tm, N}; normalization = :intensity) where {Tf, Tm, N} +function mismatch_zeroshift(fixed::AbstractArray{Tf, N}, moving::AbstractArray{Tm, N}; normalization = :intensity) where {Tf, Tm, N} size(fixed) == size(moving) || throw(DimensionMismatch("Size $(size(fixed)) of fixed is not equal to size $(size(moving)) of moving")) return _mismatch0(zero(Float64), zero(Float64), fixed, moving; normalization = normalization) end @@ -224,15 +232,15 @@ function _mismatch0(num::T, denom::T, fixed::AbstractArray{Tf, N}, moving::Abstr end """ - mm0 = mismatch0(mms::AbstractArray{<:MismatchArray}) -> NumDenom + mm0 = mismatch_zeroshift(mms::AbstractArray{<:MismatchArray}) -> NumDenom Extract and sum the zero-shift `NumDenom` from each element of an aperture-wise array-of-`MismatchArray`s, returning a single `NumDenom` representing the overall as-is mismatch. -See also: [`mismatch0(fixed, moving)`](@ref mismatch0(::AbstractArray, ::AbstractArray)). +See also: [`mismatch_zeroshift(fixed, moving)`](@ref mismatch_zeroshift(::AbstractArray, ::AbstractArray)). """ -function mismatch0(mms::AbstractArray{M}) where {M <: MismatchArray} +function mismatch_zeroshift(mms::AbstractArray{M}) where {M <: MismatchArray} mm0 = eltype(M)(0, 0) cr = eachindex(first(mms)) z = first(cr) + last(cr) # all-zeros CartesianIndex @@ -242,6 +250,8 @@ function mismatch0(mms::AbstractArray{M}) where {M <: MismatchArray} return mm0 end +Base.@deprecate mismatch0(args...; kwargs...) mismatch_zeroshift(args...; kwargs...) + """ ag = aperture_grid(ssize, gridsize) -> Array{NTuple{N,Float64},N} @@ -439,7 +449,17 @@ function truncatenoise!(mms::AbstractArray{A}, thresh::Real) where {A <: Mismatc end """ - shift = register_translate(fixed, moving, maxshift[, thresh]) -> CartesianIndex + truncatenoise(mm::AbstractArray{NumDenom{T}}, thresh) -> mm′ + truncatenoise(mms::AbstractArray{<:MismatchArray}, thresh) -> mms′ + +Non-mutating counterpart of [`truncatenoise!`](@ref); returns a thresholded copy +of the input, leaving the original unchanged. +""" +truncatenoise(mm::AbstractArray{NumDenom{T}}, thresh::Real) where {T <: Real} = truncatenoise!(copy(mm), thresh) +truncatenoise(mms::AbstractArray{<:MismatchArray}, thresh::Real) = truncatenoise!(deepcopy(mms), thresh) + +""" + shift = register_translate(fixed, moving, maxshift; thresh=nothing) -> CartesianIndex Compute the integer-valued translation that best aligns `fixed` and `moving`. All shifts up to `maxshift` (in each dimension) are considered. @@ -455,10 +475,10 @@ Returns a `CartesianIndex` of the best integer shift. Requires a concrete `mismatch` implementation to be loaded, e.g. from `RegisterMismatch` (CPU) or `RegisterMismatchCuda` (GPU). """ -function register_translate(fixed, moving, maxshift, thresh = nothing) +function register_translate(fixed, moving, maxshift; thresh=nothing) mm = mismatch(fixed, moving, maxshift) _, denom = separate(mm) - if thresh == nothing + if thresh === nothing thresh = 0.25maximum(denom) end return argmin_mismatch(mm, thresh) @@ -511,7 +531,7 @@ Compute the padded FFT size. The single-dimension form returns the smallest integer ≥ `blocksize + 2maxshift` that is efficient for FFT computation: a power of 2 along dimension 1, and a -product of `FFTPROD` primes along other dimensions. When `maxshift == 0`, +product of `FFTPROD[]` primes along other dimensions. When `maxshift == 0`, the input size is returned unchanged (that dimension will not be transformed). The multi-dimension form applies `padsize` independently to each dimension and @@ -521,21 +541,24 @@ padsize(blocksize::Dims{N}, maxshift::Dims{N}) where {N} = map(padsize, blocksiz function padsize(blocksize::Int, maxshift::Int, dim::Int) p = blocksize + 2maxshift - return maxshift > 0 ? (dim == 1 ? nextpow(2, p) : nextprod(FFTPROD, p)) : p # we won't FFT along dimensions with maxshift 0 + # dim is a 1-based dimension index; dim==1 gets pow-of-2 padding, others get nextprod(FFTPROD[],…) + return maxshift > 0 ? (dim == 1 ? nextpow(2, p) : nextprod(FFTPROD[], p)) : p end """ - assertsamesize(A, B) + checksamesize(A, B) Throw an `ErrorException` if `A` and `B` do not have the same axes along every dimension. Returns `nothing` on success. """ -function assertsamesize(A, B) +function checksamesize(A, B) return if !issamesize(A, B) error("Arrays are not the same size") end end +Base.@deprecate assertsamesize(A, B) checksamesize(A, B) + function issamesize(A::AbstractArray, B::AbstractArray) n = ndims(A) ndims(B) == n || return false @@ -614,7 +637,7 @@ unsafe_reindex(V, ::Tuple{}, ::Tuple{}) = () function padsize(blocksize, maxshift, dim) m = maxshift[dim] p = blocksize[dim] + 2m - return m > 0 ? (dim == 1 ? nextpow(2, p) : nextprod(FFTPROD, p)) : p # we won't FFT along dimensions with maxshift[i]==0 + return m > 0 ? (dim == 1 ? nextpow(2, p) : nextprod(FFTPROD[], p)) : p # we won't FFT along dimensions with maxshift[i]==0 end end #module diff --git a/test/runtests.jl b/test/runtests.jl index 5eb128e..666ee4b 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -19,12 +19,12 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma test_explicit_imports(RegisterMismatchCommon) end - @testset "set_FFTPROD" begin - old = copy(RegisterMismatchCommon.FFTPROD) - set_FFTPROD([2, 3, 5]) - @test RegisterMismatchCommon.FFTPROD == [2, 3, 5] - set_FFTPROD(old) - @test RegisterMismatchCommon.FFTPROD == old + @testset "FFTPROD" begin + old = copy(RegisterMismatchCommon.FFTPROD[]) + RegisterMismatchCommon.FFTPROD[] = [2, 3, 5] + @test RegisterMismatchCommon.FFTPROD[] == [2, 3, 5] + RegisterMismatchCommon.FFTPROD[] = old + @test RegisterMismatchCommon.FFTPROD[] == old end @testset "tovec" begin @@ -57,40 +57,46 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma @test_throws ErrorException nanpad([1.0], [1.0 2.0]) end - @testset "mismatch0 (fixed/moving)" begin + @testset "mismatch_zeroshift (fixed/moving)" begin a = [1.0 2.0; 3.0 4.0] # Identical arrays → num==0 - nd = mismatch0(a, a) + nd = mismatch_zeroshift(a, a) @test nd.num == 0.0 @test nd.denom > 0.0 # Different arrays → positive num b = [2.0 3.0; 4.0 5.0] - nd2 = mismatch0(a, b) + nd2 = mismatch_zeroshift(a, b) @test nd2.num > 0.0 # :pixels normalization - nd3 = mismatch0(a, b; normalization = :pixels) + nd3 = mismatch_zeroshift(a, b; normalization = :pixels) @test nd3.denom == 4.0 # 4 finite pixels # NaN elements are skipped c = [1.0 NaN; 3.0 4.0] - nd4 = mismatch0(a, c) + nd4 = mismatch_zeroshift(a, c) @test nd4.denom ≈ a[1,1]^2 + c[1,1]^2 + a[2,1]^2 + c[2,1]^2 + a[2,2]^2 + c[2,2]^2 # Size mismatch error - @test_throws DimensionMismatch mismatch0(a, [1.0 2.0]) + @test_throws DimensionMismatch mismatch_zeroshift(a, [1.0 2.0]) # Unrecognized normalization - @test_throws ErrorException mismatch0(a, a; normalization = :bogus) + @test_throws ErrorException mismatch_zeroshift(a, a; normalization = :bogus) + + # Deprecated alias still works + @test_deprecated mismatch0(a, a) end - @testset "mismatch0 (array-of-MismatchArrays)" begin + @testset "mismatch_zeroshift (array-of-MismatchArrays)" begin mm = MismatchArray(Float64, 3, 3) mm[0, 0] = NumDenom{Float64}(1.0, 2.0) mms = [mm] - nd = mismatch0(mms) + nd = mismatch_zeroshift(mms) @test nd == NumDenom{Float64}(1.0, 2.0) + + # Deprecated alias still works + @test_deprecated mismatch0(mms) end @testset "aperture_grid" begin @@ -151,7 +157,7 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma # dim==1 uses nextpow(2,...) @test padsize(10, 3, 1) == nextpow(2, 16) # dim==2 uses nextprod - @test padsize(10, 3, 2) == nextprod(RegisterMismatchCommon.FFTPROD, 16) + @test padsize(10, 3, 2) == nextprod(RegisterMismatchCommon.FFTPROD[], 16) # maxshift==0: no padding, no FFT @test padsize(10, 0, 1) == 10 @test padsize(10, 0, 2) == 10 @@ -171,7 +177,7 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma end end - @testset "issamesize / assertsamesize" begin + @testset "issamesize / checksamesize" begin a = zeros(3, 4) b = zeros(3, 4) c = zeros(3, 5) @@ -185,8 +191,11 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma @test !RegisterMismatchCommon.issamesize(a, (1:3, 1:5)) @test !RegisterMismatchCommon.issamesize(a, (1:3,)) - @test assertsamesize(a, b) === nothing - @test_throws ErrorException assertsamesize(a, c) + @test checksamesize(a, b) === nothing + @test_throws ErrorException checksamesize(a, c) + + # Deprecated alias still works (forwards to checksamesize) + @test_deprecated assertsamesize(a, b) end @testset "allocate_mmarrays" begin @@ -234,6 +243,43 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma end end + @testset "truncatenoise (non-mutating)" begin + mm = MismatchArray(Float64, 3, 3) + for I in eachindex(mm) + mm[I] = NumDenom{Float64}(1.0, Float64(abs(I[1]) + abs(I[2]) + 1)) + end + thresh = 2.5 + mm_orig = copy(mm) + mm_trunc = truncatenoise(mm, thresh) + @test mm_trunc !== mm + # Original untouched + for I in eachindex(mm) + @test mm[I] == mm_orig[I] + end + # Result matches in-place version + mm_inplace = truncatenoise!(copy(mm), thresh) + for I in eachindex(mm) + @test mm_trunc[I] == mm_inplace[I] + end + + # Array-of-MismatchArrays form + mms = [MismatchArray(Float64, 3, 3) for _ in 1:2] + for mm2 in mms, I in eachindex(mm2) + mm2[I] = NumDenom{Float64}(1.0, 0.5) + end + mms_orig = deepcopy(mms) + mms_trunc = truncatenoise(mms, 1.0) + @test mms_trunc !== mms + # Originals untouched + for k in eachindex(mms), I in eachindex(mms[k]) + @test mms[k][I] == mms_orig[k][I] + end + # Result was thresholded + for mm2 in mms_trunc, I in eachindex(mm2) + @test mm2[I] == NumDenom{Float64}(0.0, 0.0) + end + end + @testset "correctbias!" begin # 2D MismatchArray: row 0 and column 0 are suspect mm = MismatchArray(Float64, 5, 5) # shifts -2:2 in each dim @@ -264,6 +310,46 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma end end + @testset "correctbias (non-mutating)" begin + mm = MismatchArray(Float64, 5, 5) + good = NumDenom{Float64}(1.0, 1.0) + bad = NumDenom{Float64}(99.0, 99.0) + for I in eachindex(mm) + mm[I] = (I[1] == 0 || I[2] == 0) ? bad : good + end + mm_orig = copy(mm) + mm_corr = correctbias(mm) + @test mm_corr !== mm + # Original untouched + for I in eachindex(mm) + @test mm[I] == mm_orig[I] + end + # Result matches in-place version + mm_inplace = correctbias!(copy(mm)) + for I in eachindex(mm) + @test mm_corr[I] == mm_inplace[I] + end + + # Array-of-MismatchArrays form + mms = [MismatchArray(Float64, 5, 5) for _ in 1:2] + for mm2 in mms, I in eachindex(mm2) + mm2[I] = (I[1] == 0 || I[2] == 0) ? bad : good + end + mms_orig = deepcopy(mms) + mms_corr = correctbias(mms) + @test mms_corr !== mms + # Originals untouched + for k in eachindex(mms), I in eachindex(mms[k]) + @test mms[k][I] == mms_orig[k][I] + end + # Result matches in-place version + for k in eachindex(mms_corr), I in eachindex(mms_corr[k]) + if I[1] == 0 || I[2] == 0 + @test mms_corr[k][I] != bad + end + end + end + @testset "default_aperture_width" begin img = zeros(Float32, 20, 30) # Single cell: full image width @@ -295,4 +381,34 @@ DocMeta.setdocmeta!(RegisterMismatchCommon, :DocTestSetup, :(using RegisterMisma ov2 = RegisterMismatchCommon.computeoverlap((10, 10), (6, 6), (1, 2)) @test length(ov2) == 2 end + + @testset "register_translate (thresh shim)" begin + # Stub out the protocol mismatch method so register_translate can be exercised + # without a concrete RegisterMismatch package. + # MismatchArray(T, n, m) takes total size; for maxshift (k,k), size is (2k+1, 2k+1). + function RegisterMismatchCommon.mismatch(::Type{Float32}, + fixed::Matrix{Float32}, + moving::Matrix{Float32}, + maxshift::Dims{2}; kwargs...) + sz = 2 .* maxshift .+ 1 + mm = MismatchArray(Float32, sz...) + mm[0, 0] = NumDenom{Float32}(0.0f0, 10.0f0) # clear winner: zero mismatch, high denom + mm[1, 0] = NumDenom{Float32}(5.0f0, 10.0f0) + return mm + end + + fixed = zeros(Float32, 4, 4) + moving = zeros(Float32, 4, 4) + + # keyword form returns the expected shift + result_kw = register_translate(fixed, moving, (1, 1); thresh=0.0) + @test result_kw == CartesianIndex(0, 0) + + # default thresh (no keyword) also works + result_default = register_translate(fixed, moving, (1, 1)) + @test result_default == CartesianIndex(0, 0) + + # positional thresh form is no longer supported (removed in v1.0.2) + @test_throws MethodError register_translate(fixed, moving, (1, 1), 0.0) + end end