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
IVIM #1058
IVIM #1058
Changes from 2 commits
11b2700
951c19f
5e19275
b8570fb
49eaad3
449f119
1ce3c3d
c955cab
834c2da
00a485d
0cae14b
3f23734
f7e7823
f51b297
02c3f2c
d65b1f8
41820ac
4bac6e9
ae90bd1
927e981
762c34e
bd59df4
fc44889
27bb444
f8a89a4
c74ad47
78ba6ff
84d302d
38f4bf0
230ce3e
5437825
de22db5
a431db3
f6a9574
fe0c406
93ba045
61e4ac5
4826103
7951e6b
7885c7f
7bae1a4
d1c933b
e08d62f
4a9c1a5
0407072
8118407
f581f3e
2c8f244
d56e525
200162e
ebea630
a352e9a
a9f88e7
40e5974
24f6240
93fd782
655b3d4
88a8464
7f9ddea
a2be743
295dd88
3d56fe6
ebba0f2
89efc60
af11496
6202f99
8af6ef7
4d8b760
3babc2c
89c39ae
00c0201
4fe12ef
283d166
f28c609
98af7b7
ac34b78
3e34b1c
fbff13a
0d2fb35
a78e9d9
31a784b
625b6a8
80efaab
7d014d6
3f99e3f
5da4913
2f9134d
295f987
d38feca
ca13dee
d21d90f
3f2bdb3
a6441b2
84cce70
6913b8e
e07a989
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
#!/usr/bin/python | ||
""" Classes and functions for fitting ivim model """ | ||
from __future__ import division, print_function, absolute_import | ||
import warnings | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imported but not used |
||
import functools | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imported but not used |
||
import numpy as np | ||
from scipy.optimize import curve_fit | ||
|
||
from dipy.core.gradients import gradient_table | ||
from dipy.reconst.base import ReconstModel | ||
from dipy.reconst.dti import _min_positive_signal | ||
|
||
|
||
def ivim_function(bvals, S0, f, D_star, D): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation? |
||
S = np.vectorize(lambda b, S0, f, D_star, D: S0 * | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious from an implementation standpoint why you are using vectorize and lambda rather than straight up evaluation here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I was thinking of vectorizing the code and tried this initially. @arokem pointed out that right now I don't need to worry about that. |
||
(f * np.exp(-b * D_star) + (1 - f) * np.exp(-b * D))) | ||
return S(bvals, S0, f, D_star, D) | ||
|
||
|
||
class IvimModel(ReconstModel): | ||
"""Ivim model | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Somewhere in the docs, we will want to spell out what "IVIM" stands for. This might be a good place to do that. |
||
""" | ||
|
||
def __init__(self, gtab, fit_method="NLLS", *args, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to "inherit" a lot of its structure from the DTI model. I don't think that we need a the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I have followed the dti model and I thought that this was a good way to keep the fitting routines separate if we decide to have multiple fitting routines. If we decide to use only one routine [curve_fit or leastsq (which seems to be at the center of all fitting routines)] then I can insert the nlls_fit code here itself. I m now working on "leastsq" as a separate function similar to nlls fit. Once I complete the implementation and write the tests I can move the code in the fit method if we decide to use just one fitting routine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a key-word argument for the |
||
""" | ||
An IVIM model | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or here. |
||
|
||
Parameters | ||
---------- | ||
gtab : GradientTable class instance | ||
fit_method : Non linear least squares | ||
|
||
References | ||
---------- | ||
.. [1] Le Bihan, Denis, et al. "Separation of diffusion | ||
and perfusion in intravoxel incoherent motion MR | ||
imaging." Radiology 168.2 (1988): 497-505. | ||
.. [2] Federau, Christian, et al. "Quantitative measurement | ||
of brain perfusion with intravoxel incoherent motion | ||
MR imaging." Radiology 265.3 (2012): 874-881. | ||
""" | ||
ReconstModel.__init__(self, gtab) | ||
|
||
if not callable(fit_method): | ||
try: | ||
fit_method = common_fit_methods[fit_method] | ||
except KeyError: | ||
e_s = '"' + str(fit_method) + '" is not a known method ' | ||
raise ValueError(e_s) | ||
self.fit_method = fit_method # Add more fit methods (check dti) | ||
self.args = args | ||
self.kwargs = kwargs | ||
self.min_signal = self.kwargs.pop('min_signal', None) | ||
if self.min_signal is not None and self.min_signal <= 0: | ||
e_s = "The `min_signal` key-word argument needs to be strictly" | ||
e_s += " positive." | ||
raise ValueError(e_s) | ||
|
||
def fit(self, data, mask=None): | ||
""" Fit method of the Ivim model class | ||
|
||
Parameters | ||
---------- | ||
data : array | ||
The measured signal from one voxel. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be signal measured in more than one voxel, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @arokem The multi voxel takes care of it. While writing the documentation, I am changing this to "The measured signal." And explaining that the multivoxel decorator takes care of the rest. |
||
|
||
mask : array | ||
A boolean array used to mark the coordinates in the data that | ||
should be analyzed that has the shape data.shape[:-1] | ||
|
||
""" | ||
if mask is None: | ||
# Flatten it to 2D either way: | ||
data_in_mask = np.reshape(data, (-1, data.shape[-1])) | ||
else: | ||
# Check for valid shape of the mask | ||
if mask.shape != data.shape[:-1]: | ||
raise ValueError("Mask is not the same shape as data.") | ||
mask = np.array(mask, dtype=bool, copy=False) | ||
data_in_mask = np.reshape(data[mask], (-1, data.shape[-1])) | ||
|
||
if self.min_signal is None: | ||
min_signal = _min_positive_signal(data) | ||
else: | ||
min_signal = self.min_signal | ||
|
||
data_in_mask = np.maximum(data_in_mask, min_signal) | ||
# Lookup design matrix for vectorization ? | ||
params_in_mask = self.fit_method(data_in_mask, self.gtab) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Insert the |
||
|
||
if mask is None: | ||
out_shape = data.shape[:-1] + (-1, ) | ||
ivim_params = params_in_mask.reshape(out_shape) | ||
else: | ||
ivim_params = np.zeros(data.shape[:-1] + (4,)) | ||
ivim_params[mask, :] = params_in_mask | ||
|
||
return IvimFit(self, ivim_params) | ||
|
||
|
||
class IvimFit(object): | ||
|
||
def __init__(self, model, model_params): | ||
""" Initialize a IvimFit class instance. | ||
Parameters | ||
---------- | ||
The model parameters are S0, f, D, D_star | ||
""" | ||
self.model = model | ||
self.model_params = model_params | ||
|
||
def __getitem__(self, index): | ||
model_params = self.model_params | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is not currently being used in the tests. Please write a test that uses it. Be sure to test the corner cases (e.g., that an |
||
N = model_params.ndim | ||
if type(index) is not tuple: | ||
index = (index,) | ||
elif len(index) >= model_params.ndim: | ||
raise IndexError("IndexError: invalid index") | ||
index = index + (slice(None),) * (N - len(index)) | ||
return type(self)(self.model, model_params[index]) | ||
|
||
@property | ||
def shape(self): | ||
return self.model_params.shape[:-1] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not currently being tested. Please write a test that checks the shape attribute. |
||
|
||
|
||
def nlls_fit(data, gtab, jac=False): | ||
""" | ||
Fit the ivim params using non-linear least-squares. | ||
|
||
Parameters | ||
---------- | ||
data : array ([X, Y, Z, ...], g) | ||
Data or response variables holding the data. Note that the last | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure what "response variables holding the data" means |
||
dimension should contain the data. It makes no copies of data. | ||
|
||
jac : bool | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably drop the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we replace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. I think it would speed it up quite a bit. I'm not sure which optimization routine is best. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I replace the current fitting routine with leastsq or have a different fitting function and let the user chose the fit_method ? (for example "nlls" or "least_sq") There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think leastsq is the most flexible one. I believe one of the inputs is the algorithm so maybe the user should just input that (and have a reasonable default)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @etpeterson
Similarly from https://lmfit.github.io/lmfit-py/model.html
Curve_fit uses leastsq and allows for more flexibility. So having two fitting routines (curve_fit and leastsq) might be redundant. Also, the type of noise in IVIM data is Gaussian I presume. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd welcome input from others on this but looking at the documentation here Actually IVIM (and all MRI data) are rician. Just see the add_noise function, it defaults to rician because of that. https://github.com/nipy/dipy/blob/master/dipy/sims/voxel.py#L76. But don't worry about that, assuming the noise is gaussian is fairly safe in most cases and definitely a good place to start. If you're curious try running your tests with gaussian vs rician noise and see how it changes the fit. |
||
Use the Jacobian? Default: False | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to document all the parameters of this function |
||
Returns | ||
------- | ||
nlls_params: the S0, f, D_star, D value for each voxel. | ||
|
||
""" | ||
flat_data = data.reshape((-1, data.shape[-1])) | ||
# Flatten for the iteration over voxels: | ||
bvals = gtab.bvals | ||
ivim_params = np.empty((flat_data.shape[0], 4)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like you're letting curve_fit use ones for the initial guess. The initial guess should be reasonable values, like approximate values from the a paper for example. It also would be nice for the user to send these in if they choose. This could also be why you're getting overflows. |
||
for vox in range(flat_data.shape[0]): | ||
popt, pcov = curve_fit(ivim_function, bvals, flat_data[vox]) | ||
ivim_params[vox, :4] = popt | ||
|
||
ivim_params.shape = data.shape[:-1] + (4,) | ||
return ivim_params | ||
|
||
|
||
common_fit_methods = {'NLLS': nlls_fit} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
""" Testing DTI | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops. Please change the docstring. |
||
|
||
""" | ||
from __future__ import division, print_function, absolute_import | ||
|
||
import numpy as np | ||
from nose.tools import (assert_true, assert_equal, | ||
assert_almost_equal, assert_raises) | ||
import numpy.testing as npt | ||
from numpy.testing import (assert_array_equal, assert_array_almost_equal, | ||
assert_) | ||
import nibabel as nib | ||
|
||
import scipy.optimize as opt | ||
|
||
from dipy.reconst import ivim as ivim | ||
from dipy.reconst.ivim import ivim_function | ||
from dipy.data import get_data, dsi_voxels, get_sphere | ||
import dipy.core.gradients as grad | ||
from dipy.sims.voxel import single_tensor | ||
from dipy.io.gradients import read_bvals_bvecs | ||
from dipy.core.gradients import (gradient_table, GradientTable, | ||
gradient_table_from_bvals_bvecs, | ||
reorient_bvecs) | ||
from dipy.sims.voxel import multi_tensor | ||
|
||
|
||
def test_nlls_fit(): | ||
""" | ||
Test the implementation of NLLS | ||
""" | ||
fimg, fbvals, fbvecs = get_data('small_101D') | ||
bvals, bvecs = read_bvals_bvecs(fbvals, fbvecs) | ||
gtab = gradient_table(bvals, bvecs) | ||
|
||
f, D_star, D = 90, 0.0015, 0.0008 | ||
|
||
mevals = np.array(([D_star, D_star, D_star], [D, D, D])) | ||
# This gives an isotropic signal | ||
|
||
signal = multi_tensor(gtab, mevals, snr=None, fractions=[f, 100 - f]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you want f to be between 0 and 1, at least that's how you defined it in ivim_function. That may be why you're getting overflow errors in the fitting. Not that 0 to 100 is wrong, it just needs to be consistent. Also, f (perfusion fraction) should be around 10%, not 90%. It should fit it fine but it's just not the typical brain composition to have 90% of the signal flowing and 10% stationary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes. I am changing it to stay consistent. In multi_tensor it is taken as 0 - 100. Should I follow that or use 0 - 1 for "f" and while passing to multi_tensor do a @etpeterson Thanks for pointing it out. But even after changing it, the overflow errors persist. Can it be because the data and bvalues not what it is supposed to be for this model. (I am using 'small_101D', I 'll take a look at the data Eric posted) I think we might need tests to determine the range of f, D_star and D such that results are valid while using this model. Incorrect parameters give a very bad fit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like dipy may be a little conflicted also. Single_tensor uses S0=1 and multi_tensor uses S0=100, though others seem to prefer the 100 area. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes to make things more consistent would be most welcome! On Thu, May 26, 2016 at 9:18 AM, Eric Peterson notifications@github.com
|
||
data = signal[0] | ||
|
||
ivim_model = ivim.IvimModel(gtab) | ||
ivim_fit = ivim_model.fit(data) | ||
|
||
S0_est, f_est, D_star_est, D_est = ivim_fit.model_params | ||
est_signal = ivim.ivim_function(bvals, | ||
S0_est, | ||
f_est, | ||
D_star_est, | ||
D_est) | ||
|
||
assert_equal(est_signal.shape, data.shape) | ||
assert_array_almost_equal(est_signal, data) | ||
assert_array_almost_equal(ivim_fit.model_params, [S0, f, D_star, D]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need the shebang here, because this never gets run as a module.