Automatically shrink reconstruction volume #2221
Conversation
|
For discussion: Could this be a method on image_data
Could this be a method on acquisition_geometry
Could this be a method on acqusition_data
|
|
What should the name be
|
lauramurgatroyd
left a comment
There was a problem hiding this comment.
Hi @hrobarts
The shrinker tool is looking very useful!
I had some questions about the structure, plus some docstring and unit test suggestions, and maybe some more checking of valid parameters. Also wasn't sure about the mask parameter in the 'manual' case
| 'vertical':(bounds['vertical'][0], bounds['vertical'][1], 1)})(ig_unbinned) | ||
| return ig | ||
|
|
||
| def get_recon(self, mask_radius): |
There was a problem hiding this comment.
Would be great to have docstrings on all the methods
| recon_backend : {'tigre', 'astra'} | ||
| The backend to use for the reconstruction |
There was a problem hiding this comment.
Should mention that it uses the cil plugin, not the recon class.
Is this because the recon class doesn't yet support astra FDK?
There was a problem hiding this comment.
Yes I guess, this is just the same functionality as the COR corrector
|
|
||
|
|
||
| class TestVolumeShrinker(unittest.TestCase): | ||
|
|
There was a problem hiding this comment.
would be good to have a test with the mask parameter used
There was a problem hiding this comment.
What kind of test are you thinking? Testing the impact of the mask on the reduce_reconstruction_volume function? At the moment I'm only testing this on a very small test recon and I'm not sure how to make a mask predictably change the behaviour. I don't think this is something I have time to do but happy to add to a to-do list!
There was a problem hiding this comment.
Or do you mean test the kwargs are correctly initialised when we pass them to run?
There was a problem hiding this comment.
yes I meant the impact on the reduce_reconstruction_volume
There was a problem hiding this comment.
I guess if we had a circular mask that we know makes the region smaller than the automatic region on each dimension
|
|
||
| bounds = vs.reduce_reconstruction_volume(self.test_recon, binning=1, method='threshold', kwargs={'threshold':0.1}) | ||
| for dim in ['horizontal_x', 'horizontal_y', 'vertical']: | ||
| self.assertEqual(bounds[dim], (2,7)) |
There was a problem hiding this comment.
probably good to have a test where they aren't the same in each dimension? If not too difficult to achieve
There was a problem hiding this comment.
I'm added an asymmetrical volume
| fig, axes = plt.subplots(nrows=1, ncols=3, figsize=(8, 2.5)) | ||
|
|
||
| axes[0].imshow(arr, cmap=plt.cm.gray) | ||
| axes[0].set_title('Original') |
There was a problem hiding this comment.
Might be useful to add to the titles the direction and that its the max value (like in the other plots)
|
You will need to add the volume shrinker to this file in the documentation: |
Co-authored-by: Laura Murgatroyd <60604372+lauramurgatroyd@users.noreply.github.com> Signed-off-by: Hannah Robarts <77114597+hrobarts@users.noreply.github.com>
hrobarts
left a comment
There was a problem hiding this comment.
Hi @lauramurgatroyd thank you for your review! I think I've addressed all the comments now.
| If using 'threshold' method, specify the intensity threshold | ||
| between sample and background. Default is None. |
There was a problem hiding this comment.
Yes it fails confusingly, I've added a check now!
| mask_radius: float, optional | ||
| Radius of circular mask to apply on the reconstructed volume, before | ||
| automatically cropping the reconstruction volume. Default is None. |
There was a problem hiding this comment.
Have updated to say it's used for the cropping and in preview
| if method.lower() == 'manual': | ||
| mask_radius = kwargs.pop('mask_radius', None) | ||
| recon, binning = self.get_recon(mask_radius=mask_radius) |
There was a problem hiding this comment.
I think it's useful for the preview as well. If you had data that created a region of interest sample with a ring in the recon then it might be useful to use a mask to check how your manual limits look on the volume. I've updated the docstring to say the mask is also used for preview.
|
|
||
|
|
||
| class TestVolumeShrinker(unittest.TestCase): | ||
|
|
There was a problem hiding this comment.
What kind of test are you thinking? Testing the impact of the mask on the reduce_reconstruction_volume function? At the moment I'm only testing this on a very small test recon and I'm not sure how to make a mask predictably change the behaviour. I don't think this is something I have time to do but happy to add to a to-do list!
|
|
||
|
|
||
| class TestVolumeShrinker(unittest.TestCase): | ||
|
|
There was a problem hiding this comment.
Or do you mean test the kwargs are correctly initialised when we pass them to run?
| recon_backend : {'tigre', 'astra'} | ||
| The backend to use for the reconstruction |
There was a problem hiding this comment.
Yes I guess, this is just the same functionality as the COR corrector
| >>> cil_log_level = logging.getLogger() | ||
| >>> cil_log_level.setLevel(logging.DEBUG) | ||
| >>> vs = VolumeShrinker(data, recon_backend='astra') | ||
| >>> ig_reduced = vs.run(method='threshold' threshold=0.9) |
|
|
||
|
|
||
| class TestVolumeShrinker(unittest.TestCase): | ||
|
|
There was a problem hiding this comment.
yes I meant the impact on the reduce_reconstruction_volume
| mask_radius: float, optional | ||
| Radius of circular mask to apply on the reconstructed volume, before | ||
| automatically cropping the reconstruction volume or displaying with | ||
| preview. Default is None. |
There was a problem hiding this comment.
I think it should say here or under method that the mask_radius is not used except for the preview, for the manual case
|
|
||
|
|
||
| class TestVolumeShrinker(unittest.TestCase): | ||
|
|
There was a problem hiding this comment.
I guess if we had a circular mask that we know makes the region smaller than the automatic region on each dimension
Changes
Code to create a reduced reconstruction volume from an AcquisitionData object
If using
method='manual'limitsargument. If a dimension is not included in the limits dictionary, the full size of the axis is usedIf using
method='otsuorthresholdIf
preview=Truekwargs
bufferapplies a buffer around the automatically calculated limitsmask_radiusapplies a mask to the reconstruction before finding limitsotsu_classeschanges number of material classes to assume in the Otsu thresholdingmin_component_sizespecified a minimum number of connected components in pixels to consider when choosing the limits - to avoid a small amount of noise changing the limitsIf debug logging is enabled, we plot a histogram with the threshold and reduced box

Testing you performed
Demo TomographicImaging/CIL-Demos#283
I think it makes sense for this code to be in utilities and possibly be called by
AcquisitionData.get_ImageGeometry()however when I did this I had trouble with circular imports. Maybe it should be moved to utilities under framework?Related issues/links
May close #1998
Checklist