SyneRBI / SIRF-Contribs

Users contributions to SIRF
Apache License 2.0
2 stars 6 forks source link

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

Open mscipio opened 2 years ago

mscipio commented 2 years ago

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!

johannesmayer commented 2 years ago

Hi Michele,

thanks for reaching out.

Sharing these data will likely not be possible since they contain patientdata (even though they are not mine to share anyway).

I have a tool that will create the same file but with a moving numerical phantom inside, and then you should be able to run that code with that file instead. I will put the data into the tool and see if it is successful, then I will get back to you.

Best regards

Johannes

Johannes Mayer Quantitative MRI Physikalisch-Technische Bundesanstalt Abbestr. 2-12 10587 Berlin, Germany

@.*** phone: +49 30 3481 7233

Please visit www.ptb.de

Von: "Michele Scipioni" @.> An: "SyneRBI/SIRF-Contribs" @.> Kopie: "Subscribed" @.***> Datum: 11.02.2022 16:55 Betreff: [SyneRBI/SIRF-Contribs] Import error correction and data availability for MCIR/MR_recon_file.py contribution (Issue #8)

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! ? Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

mscipio commented 2 years ago

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

paskino commented 2 years ago

@mscipio what is the import error?

I suppose it is this line?

https://github.com/SyneRBI/SIRF-Contribs/blob/130223d9bc11991eadcd11f9b715aea34c4842fd/src/Python/sirf/contrib/MCIR/MR_recon_file.py#L21

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 commented 2 years ago

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 commented 2 years ago

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

mscipio commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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