Skip to content

Add Adapt reconstruction for SamplingOp - #47

Merged
nHackel merged 3 commits into
JuliaImageRecon:mainfrom
shukebeta:issue-22-sampling-adapt
Nov 4, 2026
Merged

nHackel merged 3 commits into
JuliaImageRecon:mainfrom
shukebeta:issue-22-sampling-adapt

Conversation

@shukebeta

Copy link
Copy Markdown
Contributor

What this PR fixes

SamplingOp returns a plain LinearOperator in both of its modes, so Adapt.adapt(target, op) cannot move it to another storage backend: the index pattern and the Boolean mask are only captured inside the operator's closures, and the image shape is not kept. This matters for MRI-style encodings such as SamplingOp ∘ FFTOp, where the FFT side can already be adapted.

How it fixes it

  • SamplingOp now returns a SamplingOpImpl <: SamplingOp{T} that wraps the existing operator and keeps the effective pattern and the shape. The numerical kernels, the public constructor, createLinearOperator, and the supported modes are unchanged. Properties are forwarded to the wrapped operator, as for WeightingOp.
  • Integer indices with shape: the stored pattern is the vectorized index vector, or the caller's vector itself for a one-dimensional pattern, as before.
  • Boolean mask: the stored pattern is the mask copy on the backend of S that the operator already used.
  • Adapt.adapt_structure and copy rebuild the operator through SamplingOp from the stored pattern. Integer indices are copied and stay on the host, as the restriction already supports. The mask is copied to the target backend. Unsupported or abstract targets raise ArgumentError through the shared target check.
  • The docstring describes both modes and Adapt.adapt.

Validation

Julia 1.10 package test suite: 1430 passed, 5 explicitly broken (single-threaded; with two threads the existing DiagOp((op, op)) FFTOp race in #44 also fails). New tests cover both modes with Float32 and ComplexF32 for Array and JLArray targets: target storage and mask placement, CPU → target, same-backend and round-trip reconstruction, source isolation (caller inputs, source pattern, and initialized workspaces), copy, the factory entry point, and SamplingOp ∘ FFTOp with its adjoint.

Four broken cases are the 5-argument adjoint mul! of the integer mode. opRestriction in LinearOperators.jl ignores α and β in its kernels (JuliaSmoothOptimizers/LinearOperators.jl#430), so that product neither scales nor accumulates. This predates the change and is left as is here. The remaining broken case is the existing NonuniformFFTs + JLArray gap.

GPU CI results will be reported separately. No Metal lane is claimed.

SamplingOp now returns a SamplingOpImpl wrapper that keeps the wrapped
operator, the effective pattern, and the shape. Adapt and copy rebuild
the operator from the effective pattern: integer indices are copied and
stay on the host, the Boolean mask is copied to the target backend.

The 5-arg adjoint product of the integer mode is marked broken in the
tests: LinearOperators' opRestriction transpose kernel ignores α and β.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.06%. Comparing base (c14b05c) to head (e7b5383).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/SamplingOp.jl 94.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
+ Coverage   81.23%   82.06%   +0.82%     
==========================================
  Files          15       15              
  Lines         613      630      +17     
==========================================
+ Hits          498      517      +19     
+ Misses        115      113       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nHackel
nHackel merged commit 93cf3b7 into JuliaImageRecon:main Nov 4, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants