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.

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

Unify data containers: CIL <=> SIRF #5

Open
mehrhardt opened this issue Jul 26, 2018 · 4 comments
Open

Unify data containers: CIL <=> SIRF #5

mehrhardt opened this issue Jul 26, 2018 · 4 comments
Assignees

Comments

@mehrhardt
Copy link

No description provided.

@mehrhardt
Copy link
Author

I encountered a new issue with the CIL-SIRF compatibility. The following example

import os
import shutil
import pSTIR as pet

os.chdir(pet.petmr_data_path('pet'))
shutil.rmtree('working_folder/thorax_single_slice',True)
shutil.copytree('thorax_single_slice','working_folder/thorax_single_slice')
os.chdir('working_folder/thorax_single_slice')

image = pet.ImageData('emission.hv');
image_array=image.as_array()*.05
image.fill(image_array);

from ccpi.optimisation.funcs import IndicatorBox
g = IndicatorBox(lower=0, upper=1)               
        
image_new = g.prox(image, 1)

results in

Traceback (most recent call last):

  File "<ipython-input-2-2381c0694166>", line 4, in <module>
    image_new = g.prox(image, 1)

  File "/home/sirfuser/devel/install/python/ccpi/optimisation/funcs.py", line 247, in prox
    return  (x.maximum(self.lower)).minimum(self.upper)

AttributeError: 'ImageData' object has no attribute 'maximum'

@paskino paskino self-assigned this Oct 17, 2018
@paskino
Copy link
Contributor

paskino commented Oct 17, 2018

Is the method maximum relevant? Should it be a method of DataContainer or could we create a function that does this job?

@KrisThielemans
Copy link
Member

We did discuss if we should add various functions like this (max/min/norm etc) and voted for not doing it to keep SIRF lightweight. Obviously, we' re adding more stuff all the time (including arithmetical operators) so I suppose it makes sense now.

If we do go ahead, I think the DataContainer should have virtual methods such that it can call the relevant function from the underlying container (e.g. STIR has find_max for images). This would be faster then first copying the stuff somehwere and then determining the max. At first sight, we could have a default implementation that calls as_array, but interestingly as_array is Python/MATLAB specific (while the C++ code obviously isn't), so I wouldn't do that. It could be done once we have iterators for DataContainers, see SyneRBI/SIRF#212.

Final thing to consider then is naming etc. You'd want to be close to the native feel. Sadly, in MATLAB max just works on 1 dimension of the array, so I suppose there is no native equivalent for maximum. numpy has amax which defaults to axis=None to use everything, but users might expect a SIRF function to be able to specify axis as well, which would really lead us to far.

Oh, and of course, it'd have to throw if the data is complex.

so, not as trivial as you'd hope.

@mehrhardt
Copy link
Author

I suggest to close this issue as we have a new one in SIRF.

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

No branches or pull requests

3 participants