[WIP] DKI ODF redux #2304

arokem · 2020-12-04T01:02:23Z

This revisits the old #685, which I think we should incorporate.

pep8speaks · 2020-12-04T01:02:29Z

Hello @arokem, Thank you for updating !

In the file dipy/reconst/tests/test_dki.py:

Line 786:1: E305 expected 2 blank lines after class or function definition, found 1
Line 786:3: E227 missing whitespace around bitwise or shift operator
Line 786:7: E225 missing whitespace around operator
Line 799:3: E225 missing whitespace around operator
Line 799:5: E225 missing whitespace around operator
Line 799:7: E225 missing whitespace around operator
Line 801:1: E302 expected 2 blank lines, found 1
Line 920:1: E305 expected 2 blank lines after class or function definition, found 0
Line 920:7: E225 missing whitespace around operator
Line 920:18: E211 whitespace before '('

Comment last updated at 2021-10-07 18:28:52 UTC

arokem · 2020-12-04T01:03:08Z

I think I got it all this time! 😅

skoudoro · 2020-12-04T01:36:37Z

Hi @arokem,

Do you plan to clean a bit this PR before a review? is it still WIP? After a really quick look, I saw some fvtk so I suppose it might have some old function too. Let us know when it is ready for review!

arokem · 2020-12-04T02:16:06Z

Yes. This is still wip!

…

On Thu, Dec 3, 2020 at 5:36 PM Serge Koudoro ***@***.***> wrote: Hi @arokem <https://github.com/arokem>, Do you plan to clean a bit this PR before a review? is it still WIP? After a really quick look, I saw some fvtk so I suppose it might have some old function too. Let us know when it is ready for review! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2304 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA46NUNSCYVQ4EVS6NPA7DSTA4LFANCNFSM4UMYHKLA> .

arokem · 2021-03-19T06:25:40Z

OK - this is no longer WIP - ready for a review!

skoudoro

Hi @arokem,

Here a quick first review. I still need to render locally the example, run the tests, and look deeper at the code.

Thanks!

skoudoro · 2021-03-19T14:33:18Z

dipy/reconst/dki.py

+ The DKI-ODF is calculated in the vertices of this input.
+ alpha : float, optional
+ radial weighting power. Default is 4 according to [Jen2014]_ and
+ [Raf2015]


missing the underscore [Raf2015]_

skoudoro · 2021-03-19T14:34:14Z

dipy/reconst/dki.py

+ normalize_peaks=False, npeaks=3, gtol=1e-5):
+ """ Estimation of fiber directions based on diffusion kurtosis imaging
+ (DKI). Fiber directions are estimated as the maxima of the DKI
+ orientation distribution function [Jen2014]. This function is based on


missing the underscore [Jen2014]_

skoudoro · 2021-03-19T14:36:53Z

dipy/reconst/dki.py

+ return dki_directions(self.model_params, sphere, alpha=4,
+ relative_peak_threshold=0.1,
+ min_separation_angle=20, mask=None,
+ return_odf=False, normalize_peaks=False,
+ npeaks=3, gtol=1e-5)


I think there is a problem here. alpha=alpha, relative_peak_threshold=relative_peak_threshold, etc... instead

skoudoro · 2021-03-19T14:38:44Z

dipy/reconst/dki.py

+ kODF = np.zeros((len(kt), len(V)))
+
+ # select non-zero voxels
+ # (when #677 is merged replace this code lines by function _positive_evals)


this is merged so this comment can be removed

skoudoro · 2021-03-19T14:39:44Z

dipy/reconst/dki.py

+ normalize_peaks=False, npeaks=3, gtol=1e-5):
+ """ Estimation of fiber direction based on diffusion kurtosis imaging
+ (DKI). Fiber directions are estimated as the maxima of the orientation
+ distribution function [Jen2014]. This function is based on the work done by


missing underscore [Jen2014]_

skoudoro · 2021-03-19T14:40:33Z

dipy/reconst/dki.py

+ relative_peak_threshold : float, optional
+ Only return peaks greater than ``relative_peak_threshold * m`` where m
+ is the largest peak.
+ min_separation_angle : float in [0, 90], optinal


typo, optional

skoudoro · 2021-03-19T14:44:06Z

doc/examples/dki_tractography.py

+affine = img.affine
+
+"""
+Having the acquition paramenters of the loaded data, we can define the


typo, parameters and acquisition

skoudoro · 2021-03-19T14:46:50Z

doc/examples/dki_tractography.py

+tensors = tensors.reshape((5, 2, 1, len(sphere.vertices)))
+tensors_actor = actor.odf_slicer(tensors, sphere=sphere, scale=0.5)
+scene.add(tensors_actor)
+window.show(scene)


doc generation will block.

if interactive=True: window.show(scene)

skoudoro · 2021-03-19T14:47:39Z

doc/examples/dki_tractography.py

+ODF = ODF.reshape((5, 1, 1, len(sphere.vertices)), order='F')
+odf_actor = actor.odf_slicer(ODF, sphere=sphere, scale=0.5)
+scene.add(tensors_actor)
+window.show(scene)


doc generation will block

if interactive=True: window.show(scene)

skoudoro · 2021-03-19T14:49:32Z

doc/examples/dki_tractography.py

+ den = img.get_fdata()
+else:
+ den = patch2self(data, gtab.bvals)
+ nib.save(nib.Nifti1Image(den, affine), 'denoised_cenir_multib.nii.gz')


could you use save_nifti function (from dipy.io.image import load_nifti, save_nifti)

skoudoro · 2021-03-19T14:54:33Z

Also, could you fix the pep8 issues

arokem · 2021-03-23T03:35:18Z

Thanks for the review @skoudoro ! I think that I've addressed the comments in the recent commit.

skoudoro · 2021-03-23T03:56:02Z

Thank you for the update!

@ShreyasFadnavis, Can you check this PR too? thank you

skoudoro

Hi @arokem,

Can you add your examples in valid_examples.txt and examples_index.rst.
Running the tutorial takes all my resources (memory, etc...) and freeze my laptop. It does not seem normal to me. So, it needs further introspection, or do you have an explanation?
Furthermore, the tutorial is super long to run! more than 1 hour on my laptop for 1 tutorial. This is really too long. there are 80 tutorials during doc generation. You might want to reduce the data size. I did not check which step take all this time.
Concerning the math part, I will let someone else have the last look.

skoudoro · 2021-03-24T15:34:32Z

doc/examples/dki_tractography.py

+for v in range(len(five_voxel_angles)):
+ angles = five_voxel_angles[v]


let's be more pythonic by using

for idx, angles in enumerate(five_voxel_angles): signal_dki[idx], ....

skoudoro · 2021-03-24T15:48:12Z

doc/examples/dki_tractography.py

+
+# convert fiber directions ground truth angles to Cartesian coordinates
+gt_dir = np.zeros((5, 1, 2, 3))
+for d in range(5):


Why we do not iterate over five_voxel_angles for more clarity

skoudoro · 2021-03-24T16:00:11Z

dipy/reconst/dki.py

+
+ Parameters
+ ----------
+ sphere : Sphere class instance, optional


optional to remove

skoudoro · 2021-03-24T16:07:34Z

dipy/reconst/dki.py

+ kODFi = kODF[rel_i]
+
+ # loop over all relevant voxels
+ for vox in range(len(kt)):


range(len( is not recommended in general. There are many alternatives more pythonic.

skoudoro · 2021-03-24T16:08:02Z

dipy/reconst/dki.py

+
+ # loop over all directions
+ sampledODF = np.zeros(len(V))
+ for i in range(len(V)):


same comment as above

range(len( is not recommended in general. There are many alternatives more pythonic.

skoudoro · 2021-03-24T16:10:19Z

dipy/reconst/dki.py

+ # loop over all relevant voxels
+ for vox in range(len(kt)):
+ # 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
+ U = np.linalg.pinv(dt) * MD
+
+ # loop over all directions
+ sampledODF = np.zeros(len(V))
+ for i in range(len(V)):
+ sampledODF[i] = _dki_odf_core(V[i], kt[vox], U, alpha)
+
+ kODFi[vox] = sampledODF


it would be good to review this block. I think this whole block can be managed more efficiently.

skoudoro · 2021-03-24T16:12:07Z

dipy/reconst/dki.py

+ normalize_peaks=False, npeaks=3, gtol=1e-5):
+ """ Estimation of fiber direction based on diffusion kurtosis imaging
+ (DKI). Fiber directions are estimated as the maxima of the orientation
+ distribution function [Jen2014]_. This function is based on the work done by


pep8: it should stop at done

skoudoro · 2021-03-24T16:37:51Z

Also, Do you plan to use this opportunity to fix #1773 (option to invert the signal in DKI WMTI)?

skoudoro · 2021-04-16T15:13:07Z

Hi @arokem,

Do you think you can finalize this for the next release, 1 week from now, or should I move it for the release in June?

arokem · 2021-04-16T16:52:11Z

Sorry. I am swamped right now. Let’s push back to next cycle. I don’t think this is too urgent.

…

On Fri, Apr 16, 2021 at 8:13 AM Serge Koudoro ***@***.***> wrote: Hi @arokem <https://github.com/arokem>, Do you think you can finalize this for the next release, 1 week from now, or should I move it for the release in June? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2304 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA46NTCYUNAUB4OADTHV7TTJBHZLANCNFSM4UMYHKLA> .

skoudoro · 2021-04-16T18:23:49Z

ok, no problem, thanks!

…Wcons which is being revised in PR dipy#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

…_odf_core

… was added on the sample of usage script.

… corrections in previous parts

…only this was computed before

…ODFs in the example

…mple of usage

skoudoro · 2022-02-22T16:17:35Z

Hi @arokem,

What is the status of this PR? How is your schedule? Do you plan to come back on it?

skoudoro changed the title ~~DKI ODF redux~~ [WIP] DKI ODF redux Dec 4, 2020

skoudoro added the type:Enhancement label Dec 4, 2020

skoudoro modified the milestone: 1.4 Dec 4, 2020

arokem force-pushed the dki_odf_redux branch from 20799b2 to 8fa35c2 Compare December 4, 2020 20:40

arokem force-pushed the dki_odf_redux branch from 68eca41 to aeedf60 Compare January 26, 2021 17:16

arokem force-pushed the dki_odf_redux branch from f7661bc to 4bc31c2 Compare March 19, 2021 04:35

arokem changed the title ~~[WIP] DKI ODF redux~~ DKI ODF redux Mar 19, 2021

skoudoro reviewed Mar 19, 2021

View reviewed changes

skoudoro added this to the 1.5 milestone Mar 22, 2021

skoudoro reviewed Mar 24, 2021

View reviewed changes

skoudoro mentioned this pull request Apr 10, 2021

[Upcoming] Release 1.4.1 #2367

Merged

17 tasks

skoudoro modified the milestones: 1.4.1, 1.5.0 Apr 16, 2021

Garyfallidis changed the title ~~DKI ODF redux~~ [WIP] DKI ODF redux Oct 7, 2021

RafaelNH added 3 commits October 7, 2021 11:26

DOC: Uppps I forgot to remove input in documentation of function _dki…

9349443

…_odf_core

DOC: Add a first draft of the example for DKI tractography usage (WIP)

0e2bfe4

RafaelNH and others added 17 commits October 7, 2021 11:26

DOC: Reconstruction and visualization of the DKI-ODF from simulations…

6a2a25a

… was added on the sample of usage script.

DOC: Start adding the real brain data part of the example and do some…

7fdb56e

… corrections in previous parts

DOC: First plot of the DKI-ODF in real data was added in the example

1c1c952

DOC, RF: Refactoring example of usage, load denoised real dataset if …

74885ec

…only this was computed before

DOC: Visualization of the fiber estimates obtain from real brain DKI-…

5a843a2

…ODFs in the example

DOC: Add steps to process all brain DKI based tractography in the exa…

cbca893

…mple of usage

Reconciling things with master branch.

d9aa170

Starting to fix outdated API.

5915735

Use patch2self for denoising.

2035700

Sometimes you get null directions even after initial checking.

7c94b91

Update to newer API.

01dc362

Converting some old fvtk calls to new fury calls.

415291d

Fixes test, by neutralizing interaction between tests in this module.

27ef917

Fixes flake8 issues in reconst/dki.py.

ba03eb7

A small additional change in the example.

9bdc726

Port the rest of the code to the new way of visualizing things.

4f0ab81

Fixes based on review comments.

c30f931

arokem force-pushed the dki_odf_redux branch from fbbebfc to c30f931 Compare October 7, 2021 18:28

arokem mentioned this pull request Mar 4, 2022

DKI Setup PennLINC/qsiprep#367

Closed

2 tasks

skoudoro force-pushed the master branch 7 times, most recently from 1419292 to ca6268a Compare December 8, 2023 22:25

skoudoro force-pushed the master branch 3 times, most recently from 5935e1e to 765963e Compare January 24, 2024 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] DKI ODF redux #2304

[WIP] DKI ODF redux #2304

arokem commented Dec 4, 2020

pep8speaks commented Dec 4, 2020 •

edited

arokem commented Dec 4, 2020

skoudoro commented Dec 4, 2020

arokem commented Dec 4, 2020 via email

arokem commented Mar 19, 2021

skoudoro left a comment

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro Mar 19, 2021

skoudoro commented Mar 19, 2021

arokem commented Mar 23, 2021

skoudoro commented Mar 23, 2021

skoudoro left a comment •

edited

skoudoro Mar 24, 2021

skoudoro Mar 24, 2021

skoudoro Mar 24, 2021

skoudoro Mar 24, 2021

skoudoro Mar 24, 2021

skoudoro Mar 24, 2021

skoudoro Mar 24, 2021

skoudoro commented Mar 24, 2021 •

edited

skoudoro commented Apr 16, 2021

arokem commented Apr 16, 2021 via email

skoudoro commented Apr 16, 2021

skoudoro commented Feb 22, 2022

		for v in range(len(five_voxel_angles)):
		angles = five_voxel_angles[v]

[WIP] DKI ODF redux #2304

Are you sure you want to change the base?

[WIP] DKI ODF redux #2304

Conversation

arokem commented Dec 4, 2020

pep8speaks commented Dec 4, 2020 • edited

Comment last updated at 2021-10-07 18:28:52 UTC

arokem commented Dec 4, 2020

skoudoro commented Dec 4, 2020

arokem commented Dec 4, 2020 via email

arokem commented Mar 19, 2021

skoudoro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skoudoro commented Mar 19, 2021

arokem commented Mar 23, 2021

skoudoro commented Mar 23, 2021

skoudoro left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skoudoro commented Mar 24, 2021 • edited

skoudoro commented Apr 16, 2021

arokem commented Apr 16, 2021 via email

skoudoro commented Apr 16, 2021

skoudoro commented Feb 22, 2022

pep8speaks commented Dec 4, 2020 •

edited

skoudoro left a comment •

edited

skoudoro commented Mar 24, 2021 •

edited