HPImaging / sv-mbirct

High Performance Model Based Image Reconstruction for Computed Tomography
BSD 3-Clause "New" or "Revised" License
6 stars 7 forks source link

Simplify MBIRReconstruction3D() interface #11

Closed cabouman closed 3 years ago

cabouman commented 3 years ago

Problem: Currently the Simplify MBIRReconstruction3D() subroutine takes in the CmdLine as an argument, but I think it only needs the verbose level. This will make porting to Cython difficult.

Solution: Parse the cmdLine and just pass the integer value of the verbose variable.

sjkisner commented 3 years ago

Fixed

smajee commented 3 years ago

To do the Cython, we need to start with a c function that inputs the sino and params returns an image.

MBIRReconstruction3D is the closest match we found, but it has other inputs like svpar, A_Padded_Map, ImageReconMask, cmdLine etc that are not currently available in the python code at the moment.

@sjkisner : Do you think there might be a better candidate c function for the Cythonization?

sjkisner commented 3 years ago

Probably most of that can be moved inside MBIRReconstruction(). It probably makes the most sense to have the main() function just handle theinterface for the command line.

How would cython pass the parameters? Can it use the same parameter structs as the c code? Can you show what the function prototype would look like?

smajee commented 3 years ago

I believe c structures defined within c header files can be imported and used via Cython. However we will create a sandbox repository with c structures using data arrays to make sure we can do this.

Assuming we are able to do the above, the MBIRReconstruct3D would need to only input structures/arrays that are present in the python code for the Cythonization to work. This can be done by moving all pieces of code in main() that do not deal with command line parsing to MBIRReconstruct3D. That way MBIRReconstruct3D would only input the sinogram data and parameters and set the image values.

However, we should have a meeting to discuss and evaluate what is required for the Cythonization before making any major changes.

sjkisner commented 3 years ago

Here are preliminary prototypes for c-routines I think we'll need. We'll need to think out the various ways these will be used by python so we can implement this properly (for example, supplying the initial error sinogram vs. not). It'll be slightly tricky if we're hiding the A matrix structure from python.

void MBIRReconstruct3D(
    float *image,  // image[i_slice*Nrows*Ncols+i_row*Ncols+i_col]
    float *sino,     // sino[i_slice*Nchannels*Ntheta +i_theta*Nchannels + i_channel]
    float *weight,
    float *proj_init,
    float *proximalmap, 
    struct ImageParams3D imgparams,
    struct SinoParams3DParallel sinoparams,
    struct ReconParams reconparams,
    char *Amatrix_fname,    // if NULL, compute internally
    char verboseLevel);

void forwardProject(
    float *proj,
    float *image,
    struct ImageParams3D imgparams,
    struct SinoParams3DParallel sinoparams,
    char *Amatrix_fname,    // if NULL, compute internally
    char verboseLevel);

void AmatrixComputeToFile(
    struct ImageParams3D imgparams,
    struct SinoParams3DParallel sinoparams,
    char *Amatrix_fname,
    char verboseLevel);
cabouman commented 3 years ago

This is great progress!

Do you need me to set up a meeting? Or should we try to figure out more via messaging first?

sjkisner commented 3 years ago

Maybe the sandbox idea for one of these prototypes is good to do first so we know more precisely how cython can interface to the c-code.

cabouman commented 3 years ago

Ok, I just set up a Cython-Sandbox.

Let me know if you want access.

smajee commented 3 years ago

@sjkisner @cabouman: Perhaps we do not need to make all the image and sinogram inputs as single indirection pointers. We can write a c wrapper to interface between what cython requires and what the current c-code has.

I have made a similar change in my branch of the cython-sandbox: https://github.com/cabouman/cython_sandbox/tree/smajee