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

Import error and data availability for MCIR/MR_recon_file.py contribution #8

Open
mscipio opened this issue Feb 11, 2022 · 8 comments

Comments

@mscipio
Copy link

mscipio commented Feb 11, 2022

Here I will be referring to MCIR/MR_recon_file.py.

I am aware that the README file states that this code is based on an old version of SIRF and that it's possible that parts of that code won't work with a recent install. For instance, most of the initial imports are not working and should be replaced with

# import engine module
import sirf.Gadgetron as pMR
import sirf.Reg as pReg

# import further modules
import numpy as np
import matplotlib.pyplot as plt

#import sys

from cil.optimisation.functions import LeastSquares, L2NormSquared, ZeroFunction, \
                                    IndicatorBox, OperatorCompositionFunction, BlockFunction
from cil.optimisation.algorithms import FISTA, CGLS, GD, PDHG
from cil.optimisation.operators import LinearOperator, CompositionOperator, BlockOperator
from cil.framework import BlockDataContainer
from ccpi.filters.regularisers import FGP_TV, TGV
from cil.framework import DataContainer as cilDataContainer

I will be happy to correct the error myself once I am able to run the code, but the data used here are not provided to the user, so the code itself is not as useful as it could be in reproducing the results of the paper.

I have just started working on a similar problem involving MCIR based on a stack-of-spiral UTE sequence, for the MR part of the protocol, and wanted to see if I could use SIRF functionality for that. It would be great to be able to start form a working example like this.

I really hope you would be open to sharing some of those data, and then I can take care to update the code so that it can run with a current version of SIRF and all the dependencies.

Best!

@mscipio mscipio changed the title Import error correction and data availability for MCIR/MR_recon_file.py contribution Import error and data availability for MCIR/MR_recon_file.py contribution Feb 11, 2022
@johannesmayer
Copy link
Member

johannesmayer commented Feb 11, 2022 via email

@mscipio
Copy link
Author

mscipio commented Feb 11, 2022

That's possibly even better!
Thanks a lot for a super quick feedback, I will be looking forward to this

@paskino
Copy link
Contributor

paskino commented Feb 11, 2022

@mscipio what is the import error?

I suppose it is this line?

from regularisers import FGP_TV, TGV

If you are only interested in the MR side of things, I would argue that SIRF master + CIL 21.3.1 should work for you, as the RPE encoding has made it into master recently.

May I ask how you are currently running or plannig to run the code?

@mscipio
Copy link
Author

mscipio commented Feb 11, 2022

The import error at least based on my recent SuperBuild installation is the fact that the current version of the MR script tries to import all the cil packages by calling them ccpi.

At a later stage, I would be interested in the PET part as well, but I would be happy for now to be able to run the MR side of the problem, and I agree that the current SIRF master should let me do it.

@paskino
Copy link
Contributor

paskino commented Feb 11, 2022

Ah, OK, so replacing ccpi with cil lets you get to the end of the imports, I should hope.

@mscipio
Copy link
Author

mscipio commented Feb 11, 2022

That, plus the fact that ccpi.plugins.regularisers now is called ccpi.filters.regularisers.
Here is a better comparison of the changes:

OLD IMPORT

from ccpi.optimisation.functions import LeastSquares, L2NormSquared, ZeroFunction, \
                                    IndicatorBox, FunctionOperatorComposition, BlockFunction
from ccpi.optimisation.algorithms import FISTA, CGLS, GradientDescent, PDHG
from ccpi.optimisation.operators import LinearOperator, CompositionOperator, BlockOperator
from ccpi.framework import BlockDataContainer

import sys
sys.path.append('/home/sirfuser/devel/buildVM/sources/CCPi-FrameworkPlugins/Wrappers/Python/ccpi/plugins/')
from regularisers import FGP_TV, TGV
from ccpi.framework import DataContainer as cilDataContainer

NEW IMPORT

from cil.optimisation.functions import LeastSquares, L2NormSquared, ZeroFunction, \
                                    IndicatorBox, OperatorCompositionFunction, BlockFunction
from cil.optimisation.algorithms import FISTA, CGLS, GD, PDHG
from cil.optimisation.operators import LinearOperator, CompositionOperator, BlockOperator
from cil.framework import BlockDataContainer
from ccpi.filters.regularisers import FGP_TV, TGV
from cil.framework import DataContainer as cilDataContainer

@ckolbPTB
Copy link

Hi, sorry I did not see your message before. Here you find a better and hopefully more up-to-date starting point for an MR-MCIR reconstruction: https://github.com/SyneRBI/SIRF-Exercises/blob/MCIR_CIL/notebooks/MR/mr_mcir_grpe.ipynb

I will send you a link with the example data to download in an email.

If you have any questions let me know!

@KrisThielemans
Copy link
Member

@ckolbPTB should we edit the README and refer to the new functionality in SIRF-Exercises? No point in duplicating.

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

5 participants