-
Notifications
You must be signed in to change notification settings - Fork 106
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.
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
Add bindings to ASTRA cylindrical detector geometries #1634
base: master
Are you sure you want to change the base?
Conversation
Currently this has to do a very unfortunate rollaxis call to permute the projection data, because the cylinder axis conventions of ASTRA and ODL don't match.
Checking new PR...
|
@@ -233,8 +236,14 @@ def _call_forward_real(self, vol_data, out=None, **kwargs): | |||
if self.geometry.ndim == 2: | |||
out[:] = self.proj_array | |||
elif self.geometry.ndim == 3: | |||
out[:] = np.swapaxes(self.proj_array, 0, 1).reshape( | |||
self.proj_space.shape) | |||
# TODO: Find a way not to have to do rollaxis(0, 3) for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use moveaxis instead? It is recommended in the docs:
https://numpy.org/doc/stable/reference/generated/numpy.rollaxis.html
vectors[:, 0:3] = geometry.src_position(angles) | ||
|
||
# Center of detector in 3D space | ||
# FIXME: This is not correct: det_point_position returns the zero-point of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, det_point_position return the center of the detector, so this is correct. The problem is with geometry.det_axes(angles), because axes are "attached" to the zero-point and not to the center as ASTRA assumes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the next few lines should look as follows:
# Center of detector in 3D space
mid_pt = geometry.det_params.mid_pt
vectors[:, 3:6] = geometry.det_point_position(angles, mid_pt)
# reverse the sign, since the partition angle in CylindricalDetector
# increases in the clockwise direction, by analogy to flat detectors
det_shift_angle = -mid_pt[0]
# `det_axes` gives shape (N, 2, 3), swap to get (2, N, 3)
det_axes = np.moveaxis(geometry.det_axes(angles + det_shift_angle), -2, 0)
The detector partition along the first dimension is given in radians, so the distance from the mid_pt[0] to the 0 point is an angle. It is sufficient to add this angle to source angles to rotate the detector axes, however the sign should be reversed, because angles increase clockwise on the detector (to be consistent with the default setup for flat detector)
This solution does not affect the situation, when the detector is positioned symmetrically. So, one could still implement the detector shift by using detector_shift_func and rotating the axes.
vectors[:, 0:3] = geometry.src_position(angles) | ||
|
||
# Center of detector in 3D space | ||
# FIXME: This is not correct: det_point_position returns the zero-point of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the next few lines should look as follows:
# Center of detector in 3D space
mid_pt = geometry.det_params.mid_pt
vectors[:, 3:6] = geometry.det_point_position(angles, mid_pt)
# reverse the sign, since the partition angle in CylindricalDetector
# increases in the clockwise direction, by analogy to flat detectors
det_shift_angle = -mid_pt[0]
# `det_axes` gives shape (N, 2, 3), swap to get (2, N, 3)
det_axes = np.moveaxis(geometry.det_axes(angles + det_shift_angle), -2, 0)
The detector partition along the first dimension is given in radians, so the distance from the mid_pt[0] to the 0 point is an angle. It is sufficient to add this angle to source angles to rotate the detector axes, however the sign should be reversed, because angles increase clockwise on the detector (to be consistent with the default setup for flat detector)
This solution does not affect the situation, when the detector is positioned symmetrically. So, one could still implement the detector shift by using detector_shift_func and rotating the axes.
This adds bindings for the
CylindricalDetector
geometry to the astra_cuda tomography backend.Known bug: there is a misinterpretion of the detector center if the detector partition isn't centered around 0, which is the case for quarter-pixel-shifted detectors. Fixing this needs to be coordinated with the Mayo CT data loader, which currently is assuming the presence of this bug.