diff --git a/API_REVIEW_PLAN.md b/API_REVIEW_PLAN.md new file mode 100644 index 0000000..cf422dc --- /dev/null +++ b/API_REVIEW_PLAN.md @@ -0,0 +1,115 @@ +# API Review Plan + + +## Metadata +- **Kind**: `api` +- **Package**: RegisterMismatchCuda +- **Source review date**: 2026-05-18 +- **Current version**: 1.0.0 + +## Stated values +Post-1.0 package; all changes must be non-breaking (target v1.0.1 patch release). T1-1 is +implemented via a deprecation shim rather than a clean break. No Tier-2 deprecation shims +needed for the remaining findings (all are purely additive or type-narrowing on constructors +that would already fail at runtime). + +## Release strategy +- **Pre-breaking-release**: `n/a (no breaking changes planned)` +- **Inter-cluster releases**: `n/a` + +## Baseline +- Tests pass on the starting commit: `assumed (clean branch be440d6)` +- `Test.detect_ambiguities` count: `not-yet-checked` +- Working tree clean: `yes` + +## Decisions +- T1-1: deprecation shim, not a clean break — keep v1.x line, bump to 1.0.1. + +## Chunks + +### CHUNK-001: preflight +- **Kind**: `preflight` +- **Originating finding**: n/a +- **Cluster**: none +- **Breaking**: no +- **Description**: Establish baseline (tests pass, ambiguity count, clean tree). +- **Depends on**: none +- **Verification**: full test suite, `Test.detect_ambiguities` +- **Status**: `completed` +- **Notes**: + +### CHUNK-002: widen-curcpair-constructor +- **Kind**: `implement` +- **Originating finding**: Tier 2 / convention 2k / `CuRCpair` +- **Cluster**: none +- **Breaking**: no +- **Description**: `CuRCpair(A::Array{T})` → `CuRCpair(A::AbstractArray{T})`. The body + calls `copyto!` which works for any `AbstractArray`; the `Array` constraint silently + rejects views and transposed arrays. +- **Depends on**: CHUNK-001 +- **Verification**: existing tests pass; optionally add a test passing a view +- **Status**: `completed` +- **Notes**: + +### CHUNK-003: fix-cmstorage-type-bounds +- **Kind**: `implement` +- **Originating finding**: Tier 3 / convention 2j / `CMStorage` +- **Cluster**: none +- **Breaking**: no +- **Description**: Display-compat constructors (lines 152–153) have `T <: Real` while the + primary constructors use `T <: AbstractFloat`. Change to `T <: AbstractFloat` so an + accidental `CMStorage{Int}(undef, ...)` call fails at the entry point rather than inside + the primary constructor. +- **Depends on**: CHUNK-001 +- **Verification**: existing tests pass; confirm `CMStorage{Int}` now errors at the outer constructor +- **Status**: `completed` +- **Notes**: + +### CHUNK-004: type-annotate-mismatch-apertures! +- **Kind**: `implement` +- **Originating finding**: Tier 3 / convention 2k / `mismatch_apertures!` +- **Cluster**: apertures-cleanup +- **Breaking**: no +- **Description**: `mismatch_apertures!` currently has no type annotations. Add + `mms::AbstractArray{<:MismatchArray}`, `fixed::CuArray`, `moving::CuArray`, + `aperture_centers::AbstractArray`, `cms::CMStorage`. This also serves as the new + canonical signature after the argument reorder in CHUNK-005. +- **Depends on**: CHUNK-001 +- **Verification**: existing tests pass +- **Status**: `completed` +- **Notes**: Must be done together with CHUNK-005 to avoid introducing a partially-typed + function with the old argument order. + +### CHUNK-005: reorder-mismatch-apertures!-args +- **Kind**: `implement` +- **Originating finding**: Tier 1 / convention 2b / `mismatch_apertures!` +- **Cluster**: apertures-cleanup +- **Breaking**: no (deprecation shim retains old order) +- **Description**: New canonical signature: `mismatch_apertures!(mms, cms, fixed, moving, + aperture_centers; normalization=:pixels)` — `cms` moves from position 5 to position 2, + consistent with `mismatch!(mm, cms, moving)`. Add a `Base.depwarn` shim for the old + order (dispatches on `cms::CMStorage` in position 5). Update the internal call in + `mismatch_apertures` (line 248) and the docstring example. +- **Depends on**: CHUNK-004 +- **Verification**: deprecation shim test; existing tests pass; check docstring example +- **Status**: `completed` +- **Notes**: + +### CHUNK-006: version-bump-1.0.1 +- **Kind**: `version-bump` +- **Originating finding**: n/a +- **Cluster**: none +- **Breaking**: no +- **Description**: Bump version to 1.0.1 in `Project.toml`. Update CHANGELOG if present. +- **Depends on**: CHUNK-002, CHUNK-003, CHUNK-004, CHUNK-005 +- **Verification**: full test suite green +- **Status**: `completed` +- **Notes**: + +## Dropped findings + + +## Session Log + + +## Open Questions diff --git a/Project.toml b/Project.toml index 909d0d2..5065541 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "RegisterMismatchCuda" uuid = "ee0d8d85-fa18-576c-9601-66ebb12862d9" -version = "1.0.0" +version = "1.0.1" authors = ["Tim Holy "] [deps] diff --git a/src/RegisterMismatchCuda.jl b/src/RegisterMismatchCuda.jl index 75400d8..199eb6f 100644 --- a/src/RegisterMismatchCuda.jl +++ b/src/RegisterMismatchCuda.jl @@ -40,7 +40,7 @@ RegisterMismatchCuda """ CuRCpair{T}(undef, realsize::Dims{N}) - CuRCpair(A::Array{T}) + CuRCpair(A::AbstractArray{T}) A paired real/complex `CuArray` sharing GPU memory, used for in-place FFT computations. @@ -49,7 +49,7 @@ complex array `C` of size `(realsize[1]÷2+1, realsize[2:end]...)` that share th underlying GPU memory (matching the layout of a real-to-complex FFT plan). The field `rng` holds `UnitRange`s indexing the unpadded region of `R`. -`CuRCpair(A::Array{T})` uploads the host array `A` into the real part of a newly +`CuRCpair(A::AbstractArray{T})` uploads the host array `A` into the real part of a newly allocated pair. Fields: @@ -73,7 +73,7 @@ function CuRCpair{T}(::UndefInitializer, realsize::Dims{N}) where {T <: Abstract return CuRCpair{T, N}(R, C, rng) end -function CuRCpair(A::Array{T}) where {T <: AbstractFloat} +function CuRCpair(A::AbstractArray{T}) where {T <: AbstractFloat} P = CuRCpair{T}(undef, size(A)) copyto!(view(P.R, P.rng...), A) return P @@ -245,7 +245,7 @@ function mismatch_apertures( (length(aperture_width) == nd && length(maxshift) == nd) || error("Dimensionality mismatch") mms = allocate_mmarrays(T, aperture_centers, maxshift) cms = CMStorage{T}(undef, aperture_width, maxshift; kwargs...) - mismatch_apertures!(mms, fixed, moving, aperture_centers, cms; normalization = normalization) + mismatch_apertures!(mms, cms, fixed, moving, aperture_centers; normalization = normalization) return mms end @@ -359,7 +359,7 @@ function mismatch!(mm::MismatchArray, cms::CMStorage{T}, moving::CuArray; normal end """ - mismatch_apertures!(mms, fixed, moving, aperture_centers, cms; + mismatch_apertures!(mms, cms, fixed, moving, aperture_centers; normalization=:pixels) -> Array{MismatchArray} Compute the mismatch between `fixed` and `moving` over a list of apertures, storing @@ -380,10 +380,17 @@ cms = CMStorage{Float32}(undef, aperture_width, maxshift) mms = allocate_mmarrays(Float32, aperture_centers, maxshift) d_fixed = CuArray(rand(Float32, 128, 128)) d_moving = CuArray(rand(Float32, 128, 128)) -mismatch_apertures!(mms, d_fixed, d_moving, aperture_centers, cms) +mismatch_apertures!(mms, cms, d_fixed, d_moving, aperture_centers) ``` """ -function mismatch_apertures!(mms, fixed, moving, aperture_centers, cms; normalization = :pixels) +function mismatch_apertures!( + mms::AbstractArray{<:MismatchArray}, + cms::CMStorage, + fixed::CuArray, + moving::CuArray, + aperture_centers::AbstractArray; + normalization = :pixels, + ) assertsamesize(fixed, moving) N = ndims(cms) for (mm, center) in zip(mms, each_point(aperture_centers)) @@ -396,6 +403,16 @@ function mismatch_apertures!(mms, fixed, moving, aperture_centers, cms; normaliz return mms end +# Deprecated argument order: (mms, fixed, moving, aperture_centers, cms) +function mismatch_apertures!(mms, fixed, moving, aperture_centers, cms::CMStorage; normalization = :pixels) + Base.depwarn( + "`mismatch_apertures!(mms, fixed, moving, aperture_centers, cms)` is deprecated. " * + "Use `mismatch_apertures!(mms, cms, fixed, moving, aperture_centers)` instead.", + :mismatch_apertures!, + ) + return mismatch_apertures!(mms, cms, fixed, moving, aperture_centers; normalization) +end + ### Utilities diff --git a/test/runtests.jl b/test/runtests.jl index 80bd94c..c5e1e04 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -159,10 +159,10 @@ end mm = RM.mismatch(A, A, maxshift) num, denom = RegisterCore.separate(mm) RegisterMismatchCommon.truncatenoise!(mm, 0.01) - @test RegisterCore.indmin_mismatch(mm, 0.01) == CartesianIndex((0, 0)) + @test RegisterCore.argmin_mismatch(mm, 0.01) == CartesianIndex((0, 0)) mm = RM.mismatch(A, B, maxshift) RegisterMismatchCommon.truncatenoise!(mm, 0.01) - @test RegisterCore.indmin_mismatch(mm, 0.01) == CartesianIndex((1, 2)) + @test RegisterCore.argmin_mismatch(mm, 0.01) == CartesianIndex((1, 2)) # Testing on more complex objects # img = rand(map(UInt8,0:255), 256, 256) @@ -176,7 +176,7 @@ end moving = map(Float32, img[rng[1] .+ 6, rng[2] .- 8]) maxshift = (10, 10) mm = RM.mismatch(fixed, moving, maxshift) - @test RegisterCore.indmin_mismatch(mm, 0.01) == CartesianIndex((-6, 8)) + @test RegisterCore.argmin_mismatch(mm, 0.01) == CartesianIndex((-6, 8)) end end end @@ -280,6 +280,30 @@ end end end +@testset "CMStorage{T,N} display-compat constructor" begin + for dev in devlist + device!(dev) do + cms = RM.CMStorage{Float32, 2}(undef, (8, 8), (2, 2); display = false) + @test eltype(cms) == Float32 + @test ndims(cms) == 2 + end + end +end + +@testset "mismatch_apertures! deprecated argument order" begin + for dev in devlist + device!(dev) do + fixed = CuArray(rand(Float32, 15, 15)) + moving = CuArray(rand(Float32, 15, 15)) + aperture_centers = [(8.0, 8.0)] + maxshift = (3, 3) + cms = RM.CMStorage{Float32}(undef, (8, 8), maxshift) + mms = [RegisterCore.MismatchArray(Float32, 7, 7)] + @test_deprecated RM.mismatch_apertures!(mms, fixed, moving, aperture_centers, cms) + end + end +end + @testset "mismatch! invalid normalization" begin for dev in devlist device!(dev) do