JSENSE estimates mps with unexpected shape

[cth7]

Hi, when using a non-cartesian trajectory, JSENSE estimates coil sensitivity maps with a shape that I didn't think was intuitive. It seems the shape of the coil sensitivity maps is determined by sp.estimate_shape(coord).

For example, when using a 3D centre-out radial trajectory that corresponds to an image size of 128, the min/max coordinates are something like

(kx_min, kx_max) = (-63.99089, 63.984524)
(ky_min, ky_max) = (-63.988026, 63.93878)
(kz_min, kz_max) = (-64.0, 63.914078)

When I give the corresponding array of k-space coordinates to sp.estimate_shape, the output estimated shape is [127, 127, 127], instead of the expected [128, 128, 128].

I'm not sure what the best fix for this is... The simplest thing to do is to change the int() function in estimate_shape to round():

shape = [round(coord[..., i].max().item() - coord[..., i].min().item())
         for i in range(ndim)]

But this will change historic results, and still might not produce the expected shape for certain trajectories.

Alternatively, all functions and classes that rely on estimate_shape can be modified so that a shape can be provided explicitly as an argument, and only when a shape isn't provided explicitly will estimate_shape be used. Functions and classes that rely on estimate_shape are:

nufft_adjoint (this function can already accept a shape explicitly)
JsenseRecon
ConvSense
ConvImage
pipe_menon_dcf

The change is relatively straight forward... but since so many different functions/classes will be changed, I thought I'd ask here first for opinions and discussion.

[kmjohnson3]

We also have this issue but just use code to adjust the scale to match as desired shape (e.g. multiple of 8/16). See below for code. I agree it might be nice to replace coords with a object that more readily allows for things like zero filling. Overall, it would be even better if there was an "encoding" class that was more flexible. We frequently have to duplicate things together to deal with upsupported encoding (for example sms).

`# Due to double FOV scale by 2
target_recon_scale = boxc_shape / img_shape
logger.info(f'Target recon scale: {target_recon_scale}')

# Scale to new FOV
target_recon_size = sp.estimate_shape(coord) * target_recon_scale

# Round to 16 for blocks and FFT
target_recon_size = 16*np.ceil( target_recon_size / 16 )

# Get the actual scale without rounding
ndim = coord.shape[-1]

with sp.get_device(coord):
    img_scale = [(2.0*target_recon_size[i]/(coord[..., i].max() - coord[..., i].min())) for i in range(ndim)]

# fix precision errors in x dir
for i in range(ndim):
    round_img_scale = round(img_scale[i], 6)
    if round_img_scale - img_scale[i] < 0:
        round_img_scale += 0.000001
    img_scale[i] = round_img_scale

logger.info(f'Target recon size: {target_recon_size}')
logger.info(f'Kspace Scale: {img_scale}')

for e in range(len(mri_raw.coords)):
    mri_raw.coords[e] *= img_scale

new_img_shape = sp.estimate_shape(mri_raw.coords[0])
print(sp.estimate_shape(mri_raw.coords[0]))`

[frankong]

Yea, I added the estimate_shape function for convenience, but definitely didn't thought it thoroughly.

I agree functions dependent on estimate_shape should include a shape explicitly as argument.
@cth7 if you want to add that, please feel free to do it and merge.
In the long run, an encoding class like @kmjohnson3 suggest might be nice too.

[cth7]

Ok I opened a PR here: #70.