Skip to content

Add Adapt reconstruction for RadonOp - #45

Open
shukebeta wants to merge 2 commits into
JuliaImageRecon:mainfrom
shukebeta:issue-14-radon-adapt
Open

shukebeta wants to merge 2 commits into
JuliaImageRecon:mainfrom
shukebeta:issue-14-radon-adapt

Conversation

@shukebeta

Copy link
Copy Markdown
Contributor

What this PR fixes

RadonOpImpl has no Adapt rule, so Adapt.adapt(target, op) cannot move a Radon operator to another storage backend. Its workspaces are built from the constructor's S, the attenuation map μ is read inside the RadonKA kernels and must live on the image backend, and the image shape is only captured by the product closures, so it cannot be recovered from the operator's fields.

How it fixes it

  • RadonOpImpl now stores the image shape (2-D or 3-D) as a field.
  • Adapt.adapt_structure(to, ::RadonOpImpl) rebuilds the operator through the public RadonOp constructor with target storage S, the stored shape and the original geometry. angles and μ go through Adapt.adapt: arrays move to the target, while ranges, scalar μ and nothing are kept. RadonKA re-uploads angles and the geometry to the image backend on every call, so only array μ must already be on the target.
  • Unsupported or abstract targets raise ArgumentError through the shared target check, as for the other operators.
  • Workspaces and closures are always fresh. On a same-backend target, angles/μ arrays may be shared with the source, as the constructor already stores them as passed. A different backend copies them.
  • Test fix: the FFTOp compound case DiagOp((source, source)) compared against the source DiagOp, which runs the shared child on both blocks in parallel and races with more than one thread. It now compares against applying the source to each block in turn, and asserts that the adapted children have independent plans and workspaces.

Validation

Julia 1.10 package test suite: 845 passed, 9 explicitly broken. New tests cover Array and JLArray targets for a 2-D operator with a non-default geometry (checked against radon/backproject directly), a 3-D operator with range angles, and array and scalar attenuation. They assert target storage, fresh workspaces, the carried-over shape and geometry, source immutability and cross-backend isolation. Forward/adjoint products match the source at rtol=1e-5 on CPU. On JLArray, RadonKA calls KernelAbstractions.synchronize, which the JLArrays backend does not implement, so those products are marked broken. The remaining broken case is the existing NonuniformFFTs + JLArray gap.

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

shukebeta and others added 2 commits October 29, 2026 20:19
Store the image shape on RadonOpImpl and rebuild the operator through
the RadonOp constructor for the target storage. Array angles and
attenuation maps are adapted to the target; ranges, scalar attenuation,
and the geometry are kept.

Also give the FFTOp DiagOp compound test distinct children, since
DiagOp evaluates them in parallel and a shared FFTOp races on its
plan buffer under multiple threads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adapt the DiagOp with the same FFTOp twice, assert the adapted children
have independent plans and workspaces, and compare against applying the
source to each block in turn, which avoids the parallel race on the
shared source child.

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

codecov Bot commented Nov 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.15%. Comparing base (454257e) to head (226697b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
+ Coverage   73.01%   73.15%   +0.13%     
==========================================
  Files          15       15              
  Lines         593      596       +3     
==========================================
+ Hits          433      436       +3     
  Misses        160      160              

☔ 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.

@shukebeta

Copy link
Copy Markdown
Contributor Author

GPU CI (Buildkite #170) is green on both CUDA and ROCm for 226697b; the new RadonOp tests pass on real GPUs. The @test_broken cases are JLArray-only, as noted in the description.

This branch has not been deployed

No deployments
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.

1 participant