Image geometry voxel size

Hackathon-000-Stochastic-QualityMetrics/ImageQualityCallback.py

Line 95 in 417a78b

voxel_size_mm = (1,1,1)):

Hi Georg, I was wondering if this should just be parsed in from the user (or from the dataset class), as this doesn't seem to be an attribute of STIR.ImageData class?

Hi Imraj, I don't know the STIR.ImageData class, but I guess there must be a methods / memeber variables that contains the voxel sizes (similar to what is in CIL's ImageData). Can you maybe ask someone who knows STIR better? I would avoid passing the voxel sizes manually since this is very prone to errors.

Thanks Georg, you're right I will have an ask about STIR

Hi Imraj, I agree with Georg that we should not pass the voxel sizes manually as in commit 417a78b because it will lead to errors down the line.

In SIRF the voxel_sizes of an ImageData object x are accessed by the method x.voxel_sizes().

@Imraj-Singh @ClaireDelplancke : I guess the most generic solution that would support ImageData from SIRF and CIL would be to do test with isinstance() and then get the voxel size in the CIL/SIRF way from the ImageData.

@gschramm @ClaireDelplancke it seems to be as simple as this:

if isinstance(ref_image,cil.framework.ImageData):
    self.voxel_size_mm = (ref_image.geometry.voxel_size_z,
                          ref_image.geometry.voxel_size_y,
                          ref_image.geometry.voxel_size_x)
elif isinstance(ref_image,sirf.STIR.ImageData):
    self.voxel_size_mm = ref_image.voxel_sizes()

Perhaps worth throwing an error if neither of those instances are true? Unfortunately, I cannot test this as at present I don't have an environment with both sirf and cil... The STFC cloud doesn't seem to have both.

Throwing an exception is definitely a good idea. If you want to test the behavior for CIL, you could simply install CIL locally from conda-forge. Or maybe ask Edo/Gemma where CIL is available on the STFC cloud (there should be sth available from the last summer school)

I've managed to test now and it all works, throwing a NotImplementedError if not sirf or cil imagedata objects. I do get a warning when using the Thorax slice:

WARNING: RadioNuclideDB::get_radionuclide: unknown modality. Returning "unknown" radionuclide.

Think this is fine though.

@gschramm hey, is it worth merging this all in?

Hi Imraj,
do you mean merging into the master branch or generating a pull request for CIL?
Georg

Hi @gschramm,

Merging the master branch I meant.

Imraj

we can merge into "main" (master), but I don't think this is necesary.
@gfardell wouldn't it make more sense to create a PR for CIL (from the georg branch directly)?

It would be good to get these changes in to CIL. We have quality_measures.py which can be updated in CIL/utilities.

If you add them to a fork of CIL and open a PR we can get them merged in. We might need to add a couple of unittests, but I can do that for you, just open a draft PR and tag me.

(1) @gfardell The ImageQualityCallback is designed in a way such that it should be able to handle CIL images and also STIR images meaning that it currently depends on cil.framework and on sirf.STIR

Hackathon-000-Stochastic-QualityMetrics/ImageQualityCallback.py

Line 2 in a481716

import cil.framework

I guess you don't want to have the sirf dependency within CIL, so would it make more sense to add the ImageQualityCallback
to SIRF?
Or is there a way we can "split" the dependencies?

(2) @Imraj-Singh everything is now merged into the main branch. I also moved the test examples to separate files and corrected 2 typos
a481716

@gschramm Are you using any methods from stir? If it's just the 'reference_image.voxel_sizes()' you could base the logic on the image data having the right attribute and avoid a stir import. Either a try/catch or python's hasattr() should work, as long as there's a comment to explain the CIL/SIRF route through the code.

thanks for the tip @gfardell
sth like this?

Hackathon-000-Stochastic-QualityMetrics/ImageQualityCallback.py

Line 114 in fef1999

if hasattr(reference_image, 'voxel_sizes'):

@Imraj-Singh : Can you try if the updated version (main branch now) still works with STIR?
If so, we can file a pull request.

Yep exactly, it looks good to me. I'm at a conference at the moment. But on Monday I can help get this in to CIL for you.

@gfardell I have forked CIL and added the ImageQualityCallback to CIL/utilities/quality_measures.py

gschramm/CIL@5fb5e68

Before creating the PR a 2 questions:

ImageQualityCallback requires a dictionary of metric functions that map two numpy 1d arrays to a scalar / array.
Examples would be mse, mae, psnr. These are already defined in CIL/utilities/quality_measures.py, but operate on data
containers. Any chance that you could make them applicable for numpy arrays. Currently that does not work since e.g.
x.abs() is called.
We have also a small script that shows how to use the ImageQualityCallback using CIL images. Can I add this somewhere / should we convert this into a test?

@gschramm

x.abs() should would for any data container as I think CIL, SIRF and numpy have this as a method. In CIL's current versions of the metrics we use norm() and normsquared() which numpy ndarrays don't have implementations of. However I think your original implementations of the metrics could be used and replace the CIL versions. We have mean(), max(), and abs() methods defined for DataContainers so they should work for CIL's DataContainers, SIRF objects or numpy arrays.

Definitely add a unit test and consider adding a simplified example usage in the docstring. See the developer's guide.

For now you can also put an example script here, I think we plan to have a nicer (slightly managed) examples script folder at somepoint.

@gfardell

I will add a unit-test and the example tomorrow
Is there any potential advantage to do the operations on the data containers instead of the underlying numpy arrays?
In the CIL psnr defition the default data range is 255. I think that is ok when dealing with 8bit photographic images, but a bad idea for all other images. Any objections in changing that default to the max() of the reference image? I think that is much better choice of the data scale when operating on floating point CT, PET, MR images that can have any scale.

@gschramm
The CIL maths operations just call numpy under the hood anyway, so it'll make no difference. The advantage is just that you can call the same function with any of the datacontainers. When passed a SIRF data container it'll use the SIRF definitions, and won't have to first extract a numpy copy of the data. Or in this case a ROI can be passed as a numpy array. So I think it just adds versatility.

I think the default value change makes sense, I'm guessing these have only been used on 8bit images before. @epapoutsellis are you happy with that?

@gfardell I added the ImageQualityCallback class to cil/utilities/quality_measures.py and now I implemented a short unit test. To run the unit test, I though I could just put my local modified CIL repo on my PYTHONPATH a la:

export PYTHONPATH=/foobar/CIL/Wrappers/Python/cil

However, when I do that I cannot start python anymore. It give the error:

Fatal Python error: init_sys_streams: can't initialize sys standard streams
Python runtime state: core initialized
Traceback (most recent call last):
  File "/foobar/CIL/Wrappers/Python/cil/io/__init__.py", line 17, in <module>
  File "/foobar/CIL/Wrappers/Python/cil/io/NEXUSDataReader.py", line 17, in <module>
ModuleNotFoundError: No module named 'numpy'

Any idea how to fix this / how to use my local modified cil python module in the unit test tests?

Possibly you should add to PYTHONPATH rather than change it to the one you want.