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

(WIP) DKI PR5 - NF: DKI-ODF estimation #685

Closed
wants to merge 27 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@RafaelNH
Contributor

RafaelNH commented Jul 20, 2015

This is the work done when I was waiting for the comments on PR #677. Basically, I am suggesting here to add the ODF estimation from DKI as proposed by Jensen et al., 2014 in DKI reconstruction module. This PR should only be merged after merging PR #677. However, I add already some points to discuss:

  • Is dipy.reconst.dki the right module for these functions or should we create another separate module ?
  • Should we create a separate example for this functions or should we pass the information from the new file dipy/doc/examples/dki_tractography.py to the previous example file in dipy/doc/examples/reconst_dki.py?

Next steps on this PR:

  • implement the formula to compute DKI-ODF for the directions given on a sphere
  • add a function test to evaluate the DKI-ODF estimation formula
  • implement a function to extract directions from this DKI-ODF

RafaelNH added some commits Jul 16, 2015

NF, DOC: Start implementing the DKI-ODF estimate. Adding dki_odf func…
…tion in DiffusionKurtosisFit class which calls the new function diffusion_kurtosis_odf. Basis of mathematical estimation will be implemented in function kodf
NF: First version of the DKI-ODF implemented, now it have to be teste…
…d (in particular the expansion using the kt symmetry)
DOC, BF: Create an example of usage from the reconstruction of the DK…
…I ODF (WIP). When using the DKI-ODF functions on the example a bug was found and fixed
The test for evaluating the dki-odf estimate depends on function dki.…
…Wcons which is being revised in PR #677. I am adding here this function to carry with thr dki-odf test. However when the 2 pull requests are merged we have to insure that duplicates of this function are removed
RF, TESTS: Remove some unnecessary input dt on functions _dki_odf_cor…
…e. Improve helper functions of the example just to make it faster
NF: Start defining a function to extract fiber directions from the DK…
…I-ODF, inputs are defined in a consistent way to dipy.reconst.peaks
NF: First version of the function to estimate fiber directions from d…
…ki was completed and tested in a single voxel simulate. For now fiber direction estimates are taken for N direction sampling the DKI-ODF
RF: Remove helper function _direction_kurtosis_odf. Now all calculati…
…ons are done in diffusion_kurtosis_odf and dki_directions. With this change we avoid some unecessary diffusion tensor format changes. Moreover, dimentionless tensor U don't need to be redefined two times in dki_directions
RF: add a close together peak removal procedure before convergence ju…
…st to remove possible duplicate peaks in opposite directions

@RafaelNH RafaelNH force-pushed the RafaelNH:DKI_PR5_dki_odf_estimation branch from b2883a2 to 13a01ef Aug 10, 2015

@@ -700,8 +800,427 @@ def split_dki_param(dki_params):
return evals, evecs, kt
def diffusion_kurtosis_odf(dki_params, sphere, alpha=4):

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

Move this function up. It's always a better practice to have functions defined or imported higher up in the file than they are called. It makes the code more readable, because you always know where to look for the import/function definition.

Parameters
----------
n : array (3,)

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

Seems like this array should have shape (3, n), where n >= 1 ?

That is, it can be more than one direction?

# Compute dimensionless tensor U
R = evecs[vox]
dt = np.dot(np.dot(R, np.diag(evals[vox])), R.T)
MD = (dt[0, 0] + dt[1, 1] + dt[2, 2]) / 3

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

I'd go with:

np.mean(np.diag(dt)) 

if that's what you are trying to do here.

# Initialize AKC matrix
V = sphere.vertices
kODF = np.zeros((len(kt), len(V)))

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

Use kt.shape[0] and V.shape[0] here

U = np.linalg.pinv(dt) * MD
# loop over all directions
sampledODF = np.zeros(len(V))

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

Same here (use V.shape)

# reshape data according to input data
kODF[rel_i] = kODFi
kODF = kODF.reshape((outshape + (len(V),)))

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

And here

# Estimate ODF
ODF = \
ODFg * (1. + 1/24. * \
(kt[0] * (3*U[0, 0]*U[0, 0] - 6*(alpha+1)*U[0, 0]*V00 + \

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

This function could be shortened substantially by making this repeated calculations into helper functions. Might improve readability.

return W
def dki_directions(dki_params, sphere, alpha=4, relative_peak_threshold=0.1,

This comment has been minimized.

@arokem

arokem Aug 17, 2015

Member

We need to think about how to make this play nicely with this function:

https://github.com/nipy/dipy/blob/master/dipy/reconst/peaks.py#L325

RafaelNH added some commits Aug 20, 2015

@RafaelNH

This comment has been minimized.

Contributor

RafaelNH commented Aug 23, 2015

Just an quick update on the work that I still have to do in this pull request:

  • Address @arokem 's comments.
  • Carefully insure that DKI based ODF functions are compatible to the changes done in PR #677 when these are merged on Dipy's main repository.
continue
# Compute dimensionless tensor U
dt = np.dot(np.dot(evecs[idx], np.diag(evals[idx])), evecs[idx].T)

This comment has been minimized.

@arokem

arokem Aug 24, 2015

Member

Maybe compute this outside of the loop? You could create a U array upfront for all voxels in one move.

# First sample of the DKI-ODF
odf = np.zeros(len(sphere.vertices))
for i in range(len(sphere.vertices)):

This comment has been minimized.

@arokem

arokem Aug 24, 2015

Member

Same here - maybe do this before going into the loop?

@RafaelNH

This comment has been minimized.

Contributor

RafaelNH commented Sep 4, 2015

FYI ...

A new article with improvements on the DKI-ODF implementation was published (Russel Glenn et al., 2015). I have to refactorize the work here according to these advances.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Dec 30, 2015

Hi @RafaelNH No news for this PR from September. Should we close it for now? Tell us what you think.

@RafaelNH

This comment has been minimized.

Contributor

RafaelNH commented Jan 10, 2016

Hi @Garyfallidis!
Yes, lets close this PR for now! I will be working on the development of other techniques in the next weeks, so I don't think there will be any problem if we close this for now.
Rafael

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 10, 2016

Okay thanks @RafaelNH . Did you see the recent discussion about the free water model? Have you contacted the person who was interesting in implementing the method?

@RafaelNH

This comment has been minimized.

Contributor

RafaelNH commented Jan 11, 2016

Hi @Garyfallidis! Yup, this will be the new step for me. I will create a new pull request with a preliminary version of the implementation of the free water model.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 12, 2016

Cool 👍

@coveralls

This comment has been minimized.

coveralls commented Apr 11, 2017

Coverage Status

Changes Unknown when pulling 19cae24 on RafaelNH:DKI_PR5_dki_odf_estimation into ** on nipy:master**.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment