BF: SFM prediction with mask

[arokem]

This fixes a bug in the way that SFM does signal prediction when fit with a mask. This is something I just stumbled upon earlier today while I was working with @dPys on nipreps/dmriprep#62. This should fix the issues we were seeing there.

[dPys]

@arokem - this is great! are there any further optimizations that can be made in runtime for the predict method of isotropic specifically? (Or maybe a better question would be what particular lines take the longest to run at the moment?)

[codecov]

Codecov Report

Merging #2041 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2041      +/-   ##
==========================================
+ Coverage   91.13%   91.15%   +0.01%     
==========================================
  Files         246      246              
  Lines       31546    31611      +65     
  Branches     3307     3314       +7     
==========================================
+ Hits        28750    28815      +65     
  Misses       2081     2081              
  Partials      715      715

Impacted Files	Coverage Δ
dipy/reconst/tests/test_sfm.py	98.71% <100%> (+0.48%)	⬆️
dipy/reconst/sfm.py	91.97% <100%> (+1.06%)	⬆️

[arokem]

@dPys : without a doubt, the most time is spent in this line: https://github.com/dipy/dipy/blob/master/dipy/reconst/sfm.py#L433, which is where sklearn's ElasticNet fit is being called. Unfortunately, I don't think there's much we can do about it.

[dPys]

@arokem -

A few more questions:

1. what aspect(s) of signal prediction would be lost when reducing the ratio of L1 penalty (i.e. closer to if not equal to 0)?

2. is the estimator fit at every voxel in the ROI? what other data dimensionality reduction strategies might improve fit and/or increase the speed of prediction? Currently the prediction with this model is probably much too slow to lend to predicting from every vector coordinate in the case of head motion correction.

3. maybe related to (2) — could a T1w image, registered to the B0 template, be incorporated into this somehow?

And curious to hear from anyone else who might want to chime in.

[arokem]

what aspect(s) of signal prediction would be lost when reducing the ratio of L1 penalty (i.e. closer to if not equal to 0)?

That depends on the data. In my experience, signal prediction can be almost equally accurate with various ratios, leading to quite different fODFs.

is the estimator fit at every voxel in the ROI? what other data dimensionality reduction strategies might improve fit and/or increase the speed of prediction? Currently the prediction with this model is probably much too slow to lend to predicting from every vector coordinate in the case of head motion correction.

Yes. As suggested by @mattcieslak, one way to make this go much faster is to provide an initialized sklearn Ridge object as the input to the solver keyword argument when initializing the SparseFascicleModel object.

maybe related to (2) — could a T1w image, registered to the B0 template, be incorporated into this somehow?

I am not sure. There might be different levels of regularization that work better in different tissue types, but it would require quite a bit of empirical work to sort this out. I would be inclined to first try one ridge regularization parameter everywhere and see how that works.

[skoudoro]

Hi @arokem, Overall it looks good but there is only one thing I do not get. See below.

[skoudoro]

I do not understand this part. Can you tell us more about this part?

[arokem]

Yes: this is just to deal with the (rather unusual) case that your data is a one-dimensional vector representing one voxel's worth of data. In that case, data.shape[:-1] is () and you might run into trouble here.

[skoudoro]

ok, I see, thanks. I wonder if there is a better way to do that but It is fine like this too

[skoudoro]

Anyone to review this PR? Do you want to have a last look @dPys?

I will wait until Sunday to merge it.

[skoudoro]

ok, Thanks @arokem, merging

[dPys]

Hi @skoudoro -- sorry to chime back in late this convo, but thanks for merging. I took another look at all of @arokem 's edits and they indeed looked good to go.

Cheers,
@dPys