NF: rumba reconst #2423

kenjimarshall · 2021-07-22T14:14:35Z

Robust and Unbiased Model-BAsed Spherical Deconvolution (RUMBA-SD)

RumbaSD and RumbaFit classes for local reconstruction
Standalone global_fit method for whole brain fitting with built-in spatial regularization option
Examples for both local reconstruction (single- and multi-shell) and fiber tracking with RUMBA-SD
Model compartments for white matter, grey matter, and cerebrospinal fluid

Developed in collaboration with @gabknight and @ejcanalesr. RUMBA-SD is the algorithm that won the MICCAI IronTract 2019 challenge, the 3D VoTEM ISBI 2018 challenge (sub-challenge #2), and the co-winner of the ISBI HARDI 2013 challenge.

Reference:

Canales-Rodríguez, E. J., Daducci, A., Sotiropoulos, S. N., Caruyer, E., Aja-Fernández, S., Radua, J., Mendizabal, J. M. Y., Iturria-Medina, Y., Melie-García, L., Alemán-Gómez, Y., Thiran, J.-P., Sarró, S., Pomarol-Clotet, E., & Salvador, R. (2015). Spherical Deconvolution of Multichannel Diffusion MRI Data with Non-Gaussian Noise Models and Spatial Regularization. PLOS ONE, 10(10), e0138910. https://doi.org/10.1371/journal.pone.0138910

…ic compartments

…AB implementation

…e 3D (not 4D)

Decompose into rotation and scaling components also in case the affine is a 3x3 matrix

Co-authored-by: Ariel Rokem <arokem@gmail.com>

kenjimarshall · 2021-10-08T15:40:00Z

Yes that works! I'll see you then.

kenjimarshall · 2021-10-15T15:06:52Z

Hi @arokem the changes we discussed yesterday have all been pushed!

The only difference from our conversation is that I ended up having to define _voxelwise_fit and _global_fit within the RumbaSD rather than outside, so I'm doing the unpleasant self.fit = self._voxelwise_fit assignment. This is because, if defined outside, then model needs to be passed explicitly whenever fit is called. This breaks the typical fit signature. Let me know if this is okay, or if there's a solution to this that I didn't think of.

Thank you!

arokem

Looks good! I had a couple of suggestions here and there.

I also think that we should change the name of the "RumbaSD" class to "RumbaSDModel" to be consistent with the other models.

dipy/reconst/tests/test_rumba.py

doc/examples/reconst_rumba.py

doc/examples/reconst_rumba_multishell.py

kenjimarshall · 2021-10-21T13:50:24Z

Hi @arokem I pushed these suggestions. I'm not sure why all the Linux checks were cancelled... Should I try making another commit to re-run the tests?

skoudoro · 2021-10-21T14:21:17Z

Hi @kenjimarshall,

They failed because ubuntu 1604 is deprecated. We need to update our azure-pipeline files. See #2413

arokem · 2022-02-10T22:15:31Z

Following up here, after our discussion this morning. I think that it was suggested that it would be OK to merge this code into master, but there was some question about how to discuss the multi-shell implementation in the example. One easy way to deal with the questions around multi-shell would be to remove that example. I think that maybe @gabknight was suggesting that as a possibility. I'd alright with that. What do others think?

ejcanalesr · 2022-02-11T08:41:49Z

Hi Ariel/dipy team, I agree, we could add the multi-shell example in the near future. Thanks! All the best, Erick

…

________________________________ From: Ariel Rokem ***@***.***> Sent: Thursday, February 10, 2022 11:15 PM To: dipy/dipy Cc: Canales Rodriguez Erick Jorge; Mention Subject: Re: [dipy/dipy] NF: rumba reconst (#2423) Following up here, after our discussion this morning. I think that it was suggested that it would be OK to merge this code into master, but there was some question about how to discuss the multi-shell implementation in the example. One easy way to deal with the questions around multi-shell would be to remove that example. I think that maybe @gabknight<https://github.com/gabknight> was suggesting that as a possibility. I'd alright with that. What do others think? — Reply to this email directly, view it on GitHub<#2423 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACFIXLJYTIIRMLWNTDALKETU2Q2I3ANCNFSM5A2DGRVQ>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

arokem · 2022-02-11T19:12:31Z

Hi @kenjimarshall : are you still available to work on this? Could you make this change? I think after removing the multi-shell example, we'd be ready to merge this.

kenjimarshall · 2022-02-11T19:15:16Z

Hi @arokem and everyone. Yes no problem! I'll delete it now.

To clarify, we're keeping the multi-shell functionality on the model but just deleting the example?

arokem · 2022-02-11T19:18:31Z

Thanks for being so quick! I think that this is ready for the merge. I will wait until EOB Monday, to give people time to raise any objections, before I go ahead with it.

doc/examples/reconst_rumba.py

Garyfallidis · 2022-02-14T18:51:40Z

Hi @kenjimarshall and all. This PR needs rebasing.

Garyfallidis

The PR needs a careful rebasing and a few more comments need addressing too. Overall, a great work in the making :) This is super close to be merged.

dipy/reconst/rumba.py

Garyfallidis · 2022-02-14T18:35:54Z

dipy/reconst/rumba.py

+    matter (WM) fiber populations in a given orientation as well as effects
+    from GM and CSF. The equation governing these  contributions is:
+
+    $ S_i = S_0\left(\sum_{j=1}^{M}f_j\exp(-b_i\bold{v}_i^T\bold{D}_j


@kenjimarshall have you tried to render the documentation? If yes, do the math render correctly?

Thanks for pointing this out -- there were some rendering issues that I believe I've now fixed.

Garyfallidis · 2022-02-14T18:41:40Z

dipy/reconst/tests/test_rumba.py

+    bvecs = btable[:, 1:]
+    gtab = gradient_table(bvals, bvecs)
+
+    rumba = RumbaSDModel(gtab, n_iter=600, sphere=sphere)


Is this model okay to use with DSI type of data?

Yes I believe so, but maybe @ejcanalesr can confirm?

Garyfallidis · 2022-02-14T18:42:18Z

dipy/reconst/tests/test_rumba.py

+    data_pred = rumba_fit.predict()
+
+    # Assert reconstructed signal value within 0.01 of original
+    assert_allclose(data, data_pred, atol=0.01, rtol=1.0)


Tolerance seem low. Any reasons?

I just increased the precision on this test a bit further. It's now close to the threshold for RUMBA's predict performance.

doc/examples/reconst_rumba.py

Garyfallidis · 2022-02-14T19:21:08Z

@arokem have you checked that the documentation of this PR renders correctly?

Garyfallidis · 2022-02-17T19:48:45Z

Thank you for the great work @kenjimarshall and team. I am merging this now.

Garyfallidis · 2022-02-17T19:49:33Z

🎉

kenjimarshall and others added 30 commits June 30, 2021 10:44

First implementation of rumba module

d00bb2f

Add input argument checks to RumbaSD class and global_fit function

3c8e3f8

Use DIPY's normalize_data function rather than our implementation

f8c9bd6

Make diffusivity arguments optional with reasonable defaults

3f14c28

Add f_csf and f_gm volume fraction maps as properties to RumbaFit

1473b64

Make fitting of isotropic compartments optional

9d70866

Generate warning if fitting with one b-value and wanting both isotrop…

999879b

…ic compartments

Change gradient and divergence implementations to match original MATL…

f1eec85

…AB implementation

Modify dimensionality of returned isotropic volume fraction maps to b…

7e68d64

…e 3D (not 4D)

Remove grey matter compartment -- found to only fit noise

dee8e0c

Remove old divergence implementation

8614572

Update gradients.py

bcf6f30

Decompose into rotation and scaling components also in case the affine is a 3x3 matrix

Update gradients.py

55bfb5e

Add test for shear in reorient_bvecs

10ca2f2

pep control

b8d4ec4

simplify test

f92d47e

Co-authored-by: Ariel Rokem <arokem@gmail.com>

from random to deterministic test

d8886c7

BF: KFA is set to zero if MKT is 0 or negative

538be34

PEP8: Fix some style typos in updated files

0ffe3d1

BF: KFA is zero for small MKT values (MKT < tol, tol=1e-8)

626de3f

NF - get the pmf for an direction

dcce601

TST - get the pmf for an direction

b54f1f8

RF - moved the sphere attribute of SHCoeffPmfGen to the parent class

66fa824

DOC - update api_change

a2bb3bd

update declaration and remove _c function

9ed2593

default sphere + remove get_pmf_c

37843de

update tests

36cc6ae

RF - removed _c function from BootPmfGen

f1312f4

TST - pmf sphere ValueError

e44c8d5

RF - set the pmf sphere parameter as mandatory

3e41191

kenjimarshall added 3 commits October 12, 2021 22:51

Fit redesign and prediction function

3d18197

Remove odf parameter in predict

cbe48e9

Reorganize fit, remove multi_voxel_fit decorator, add predict test

3c9cc54

arokem reviewed Oct 19, 2021

View reviewed changes

Rename to RumbaSDModel, other minor changes

4791067

skoudoro mentioned this pull request Oct 29, 2021

[Upcoming] Release 1.5.0 #2462

Merged

23 tasks

Remove multishell example

d1b4f15

Remove references to multi-shell example

e429e74

Garyfallidis reviewed Feb 14, 2022

View reviewed changes

doc/examples/reconst_rumba.py Outdated Show resolved Hide resolved

Garyfallidis requested changes Feb 14, 2022

View reviewed changes

skoudoro added the type:New feature label Feb 14, 2022

kenjimarshall added 5 commits February 14, 2022 16:22

Fix equation rendering

c6adff7

Fix documentation typos

0df088e

Constrain num_processes in example

b252e4d

Increase precision of test_predict

f31cbd3

Fix typos in example

6166dc5

Garyfallidis merged commit d15715b into dipy:master Feb 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NF: rumba reconst #2423

NF: rumba reconst #2423

kenjimarshall commented Jul 22, 2021 •

edited

kenjimarshall commented Oct 8, 2021

kenjimarshall commented Oct 15, 2021

arokem left a comment

kenjimarshall commented Oct 21, 2021 •

edited

skoudoro commented Oct 21, 2021

arokem commented Feb 10, 2022

ejcanalesr commented Feb 11, 2022 via email

arokem commented Feb 11, 2022

kenjimarshall commented Feb 11, 2022 •

edited

arokem commented Feb 11, 2022

Garyfallidis commented Feb 14, 2022

Garyfallidis left a comment

Garyfallidis Feb 14, 2022

kenjimarshall Feb 14, 2022

Garyfallidis Feb 14, 2022

kenjimarshall Feb 14, 2022

Garyfallidis Feb 14, 2022

kenjimarshall Feb 14, 2022

Garyfallidis commented Feb 14, 2022

Garyfallidis commented Feb 17, 2022

Garyfallidis commented Feb 17, 2022

NF: rumba reconst #2423

NF: rumba reconst #2423

Conversation

kenjimarshall commented Jul 22, 2021 • edited

kenjimarshall commented Oct 8, 2021

kenjimarshall commented Oct 15, 2021

arokem left a comment

Choose a reason for hiding this comment

kenjimarshall commented Oct 21, 2021 • edited

skoudoro commented Oct 21, 2021

arokem commented Feb 10, 2022

ejcanalesr commented Feb 11, 2022 via email

arokem commented Feb 11, 2022

kenjimarshall commented Feb 11, 2022 • edited

arokem commented Feb 11, 2022

Garyfallidis commented Feb 14, 2022

Garyfallidis left a comment

Choose a reason for hiding this comment

Garyfallidis Feb 14, 2022

Choose a reason for hiding this comment

kenjimarshall Feb 14, 2022

Choose a reason for hiding this comment

Garyfallidis Feb 14, 2022

Choose a reason for hiding this comment

kenjimarshall Feb 14, 2022

Choose a reason for hiding this comment

Garyfallidis Feb 14, 2022

Choose a reason for hiding this comment

kenjimarshall Feb 14, 2022

Choose a reason for hiding this comment

Garyfallidis commented Feb 14, 2022

Garyfallidis commented Feb 17, 2022

Garyfallidis commented Feb 17, 2022

kenjimarshall commented Jul 22, 2021 •

edited

kenjimarshall commented Oct 21, 2021 •

edited

kenjimarshall commented Feb 11, 2022 •

edited