SyneRBI / SIRF-Exercises

SIRF Training and demonstration material
http://www.ccpsynerbi.ac.uk
Apache License 2.0
18 stars 21 forks source link

MR-joint-TV demo needs fixing #54

Closed DANAJK closed 3 years ago

DANAJK commented 4 years ago

At least running via Docker, this demo seems to be missing a directory:

OSErrorTraceback (most recent call last)
<ipython-input-2-1f29d02172b2> in <module>()
     14 #%% GO TO MR FOLDER
     15 os.chdir(examples_data_path('MR'))
---> 16 os.chdir('johannes')

OSError: [Errno 2] No such file or directory: 'johannes'
KrisThielemans commented 4 years ago

good spot. In fact, this demo is currently still broken, as it says. It's probably not a bad thing that it fails at the start, as the algorithm is wrong anyway...

I guess that we could fix it with new CIL (@paskino is that correct?). But we don't have data for it at present either I believe, which is why we downloaded it during the exercise from Johannes somewhere. @johannesmayer, could you confirm that this (or similar) data is not yet on zenodo?

For now, I will just edit the text.

KrisThielemans commented 4 years ago

updated in c9380fe65ecd44eefec62b7a3b25a13a6ffa0b5d.

I will keep the issue open to remind us to fix this later.

mscipio commented 4 years ago

Even though this notebook is not yet functional, I was trying to take inspiration from the idea of a regularized MR reconstruction to build a separate test case. However, when at the point of defining Gradients, it seems that the current version of ccpi does not ship with a method called :

from ccpi.optimisation.operators import GradientSIRF

If I try to replace it with the only other option available:

from ccpi.optimisation.operators import Gradient

I get the following error: image

My guess is that the GradientSIRF function should act as a wrapper, providing CIL with the necessary info about the geometry.

Is there any WIP version of ccpi where I can find an example of GradientSIRF? Or, better, are there any other strategies I can use within SIRF to define the gradient to build my own regularized MR reconstruction, without having to write it by hand, from scratch?

KrisThielemans commented 3 years ago

@paskino @epapoutsellis @gemm can you remind us what the current status is of the gradient operators?

epapoutsellis commented 3 years ago

For CIL objects, please use from cil.optimisation.operators import GradientOperator. For SIRF objects I had a GradientSIRF implementation which I cannot find atm. Maybe @paskino knows. The other option is to use an FGP_TV solver, where the gradient is computed in a C level, so not worry about sirf/cil objects

KrisThielemans commented 3 years ago

@Pakino wrote

I think that since a while CIL GradientOperator can be used straight away with a SIRF DataContainer, no need for obscure workarounds.

paskino commented 3 years ago

I believe that this notebook is superseded by cil_joint_tv_mr.ipynb and this issue should be closed @KrisThielemans @DANAJK

@mscipio I believe that the current version of CIL supports natively the GradientOperator on SIRF DataContainers.

KrisThielemans commented 3 years ago

I agree. @ckolbPTB could confirm.

KrisThielemans commented 3 years ago

obviously, the obsolete notebook should be removed

ckolbPTB commented 3 years ago

yes, this notebook can be deleted.