Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Resolve #530 #531

Merged
merged 4 commits into from
Jun 12, 2024
Merged

Resolve #530 #531

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e8651aa
Add a test for fbp method
bwohlberg Jun 7, 2024
43a7e8b
Add support for fbp on gpu
bwohlberg Jun 7, 2024
013fd6f
Add comment to test as per PR review
bwohlberg Jun 12, 2024
5786ce6
Merge branch 'main' into brendt/issue530
bwohlberg Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions scico/linop/xray/astra.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ def fbp(self, sino: jax.Array, filter_type: str = "Ram-Lak") -> jax.Array:
<https://www.astra-toolbox.com/docs/algs/FBP_CUDA.html>`__.
"""

# Just use the CPU FBP alg for now; hitting memory issues with GPU one.
def f(sino):
sino = _ensure_writeable(sino)
sino_id = astra.data2d.create("-sino", self.proj_geom, sino)
Expand All @@ -200,7 +199,7 @@ def f(sino):
rec_id = astra.data2d.create("-vol", self.vol_geom)

# start to populate config
cfg = astra.astra_dict("FBP")
cfg = astra.astra_dict("FBP_CUDA" if self.device == "gpu" else "FBP")
cfg["ReconstructionDataId"] = rec_id
cfg["ProjectorId"] = self.proj_id
cfg["ProjectionDataId"] = sino_id
Expand Down
5 changes: 5 additions & 0 deletions scico/test/linop/xray/test_astra.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ def test_adjoint_typical_input(testobj):
adjoint_test(A, x=x, rtol=get_tol())


def test_fbp(testobj):
x = testobj.A.fbp(testobj.y)
assert np.sum(np.abs(x)) > 0.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider checking that x is reasonably close to testobj.x.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That did occur to me, but I decided against it because it wasn't obvious how to choose a reasonable threshold for "reasonably close". But easy to add if you think it's nevertheless worthwhile.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On further consideration, I think what I'd prefer is a comment to the effect of "checks for bug where fbp would return all zeros when because ASTRA CPU FBP algorithm was being called on a GPU system."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Please confirm approval.



def test_jit_in_DiagonalStack():
"""See https://github.com/lanl/scico/issues/331"""
N = 10
Expand Down
Loading