Skip to content
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

Single-shell Free Water DTI (Object Oriented) #2087

Open
wants to merge 60 commits into
base: master
Choose a base branch
from

Conversation

mvgolub
Copy link

@mvgolub mvgolub commented Mar 13, 2020

This is my third pull request for single-shell FW-DTI. The previous PR's (#1603 and #1744) were abandoned because many changes to the code were made since then.

The main difference of this code is that it was implemented with objects and classes, to be consistent with Dipy's implementations of other reconstruction models; also all functions were written inside the existing dipy/reconst/fwdti.py module, instead of being implemented in a separate file.

This code is based on a regularized gradient descent approach proposed by [1], but instead uses an euclidean metric for the regularization, as proposed in [2]. Additionally, a more novel initialization method for the FW fraction was implemented, proposed in [3].

The code is not ready to be merged, needs to be reviewed @RafaelNH. When reviewing, please ignore all commits made before 2020, I accidentally branched off from an old feature branch and the old outdated commits appear in this PR, I did not know how to fix it (begginer's mistake, sorry).

TODO:

  1. Add unit tests
  2. Add a wrapper to process data by chunks. Currently the code is not optimized to run on large data sets. This code is vectorized, processing the entire data at once. Unlike standard DTI and multi-shell FW-DTI, there is no single-voxel fit and it is not possible to process linearized / flattened chunks of data, because this method requires spatial derivatives to perform regularization, only possible on 3D / 2D multi-voxel data. Maybe it is possible?
  3. Add more theory in DOC strings
  4. Correct PEP8 errors

References:
[1] Pasternak, O., Sochen, N., Gur, Y., Intrator, N., & Assaf, Y. (2009). Free water elimination and mapping from diffusion MRI. Magnetic Resonance in Medicine: An Official Journal of the International Society for Magnetic Resonance in Medicine, 62(3), 717-730. (https://onlinelibrary.wiley.com/doi/full/10.1002/mrm.22055)

[2] Pasternak, O., Maier-Hein, K., Baumgartner, C., Shenton, M. E., Rathi, Y., & Westin, C. F. (2014). The estimation of free-water corrected diffusion tensors. In Visualization and Processing of Tensors and Higher Order Descriptors for Multi-valued Data (pp. 249-270). Springer, Berlin, Heidelberg. (https://link.springer.com/chapter/10.1007/978-3-642-54301-2_11)

[3] Ismail, A. A. O., Parker, D., Hernandez-Fernandez, M., Brem, S., Alexander, S., Pasternak, O., ... & Verma, R. (2018, September). Characterizing peritumoral tissue using DTI-Based free water elimination. In International MICCAI Brainlesion Workshop (pp. 123-131). Springer, Cham. (https://www.hal.inserm.fr/inserm-01867347/file/BrainLes-FWE-2018.pdf)

…ions necessary to perform Beltrami regularization. Added functions that convert the diffusion tensor components to the Iwasawa coordinate system. Also added the respective test functions to test_beltrami.py
…ld and d_manifold functions, to match the order used in dipys functions
1) removed redundant masking operations.
2) modified the functions to return an output (no need for pre-allocation).
Forward and backward differences of the Iwasawa coordinates will be used to compute the induced metric.
This function computes the Beltrami operator increments when the chosen metric is euclidean.
These functions compute the Levi-Civita terms for each Iwasawa coordinate,
these terms are necessary to compute the Beltrami operator when the affine
metric is chosen instead of the euclidean metric.
This function computes the Beltrami and Levi-Civita incremnts to update the
Iwasawa coordinates when the affine metric is chosen, the computations are
more complex than those of the euclidean metric.

Note: When the euclidean metric is chosen, the beltrami_euclidean function
is called instead. This function acts directly on the diffusion tensor
coordinates (no need for Iwasawa conversion) and there is no need to compute
the Levi-Civita terms.
This function computes the Fidelity increments to update the diffusion
tensor components, it also computes the increment to update the tissue
volume fraction.

Note: Similarly to beltrami_euclidean, fidelity_euclidean acts directly on
the diffusion tensor data (no Iwasawa parameterization) and the operations
involved are simpler than its affine counterpart.
These auxiliary functions compute the matrix dot product q*DXi*q, where q is
a single gradient direction and DXi denotes the diffusion tensor expressed
in terms of the Iwasawa parameterization, differentiated with respect to Xi
(the ith Iwasawa coordinate). The derivative is symbolic.

These dot products are necessary to compute the Fidelity increments when the
affine metric is chosen over the euclidean.

Note: These functions compute the q*DXi*q products for all q.
This function computes the Fidelity increments to update the Iwasawa
coordinates when the affine metric is chosen over the euclidean. This version
of the fidelity function requires the symbolic derivatives of the diffusion
tensor with respect to each Iwasawa coordinate, which are computed with the
get_qDXiq functions (added in the previous commit).
This file calls the functions from reconst/beltrami.py to implemnt the main
gradient descent routine and estimate the Free Water fraction, also, it deals
with pre-processing of the raw data and initialization of the diffusion
tensor and free water fraction.

Note: Eventually the fucntions from this file should be integrated into the
reconst/fwdti.py module.
This function deals with pre-processing the raw data and returns the normalized
attenuations, which are necessary to initialize the diffusion tensor and free water
fraction before performing the Beltrami routine.
This function estimates the initial guess for the diffusion tensor and tissue volume
fraction, which are necessary before performing the Beltrami routine.
This is the main loop that performs the Beltrami routine and estimates the tissue
volume fraction and corrected diffusion tensor, it combines all previously implemented
functions in beltrami.py
This class treats the diffusion parameters as a manifold and holds the necessary methods and metrics to perfrom regularized gradient descent on the diffusion parameters.
These methods are ncessary to compute the euclidean metric tensor,
which in turn is used to compute the Laplace-Beltrami operator.
This method computes the regularization term to smooth
the diffusion tensor, using its spatial derivatives.
The fidelity term moves the parameters aginst the gardient direction,
i.e. towards the minimum of the cost function.

The cost function is implemented to debug and guarantee that the
gradient descent routine converges.
The 'update' method increments all parameters of the FW-DTI model,
using the Laplace-Beltrami and fidelity terms.

Also added property to Manifold class, that outputs the parameters
in a form consistent with dipy.
These classes are the equivalent to FreeWtaerTensorModel and
FreeWaterTensorFit but for single-shell data. Maybe calling them Beltrami
is misleading, since both cases fit the FW-DTI model, Beltrami refers to the
regularization of the Laplace-Beltrami operator, maybe change names in the
future.
Also added extra properties to BeltramiFit, allowing quik access to
the initial FW, MD, FA and FW limits
This fucntion takes the raw data and returns normalized attenuations without
multiple b0 data
This function initializes the tissue fraction and its bounds based on the
S0 image.
This function initializes the tissue fraction based on the initial
MD map, estimated with standard DTI
@mvgolub
Copy link
Author

mvgolub commented Jun 11, 2021

Hi @mvgolub and @RafaelNH,

Can we get a quick feedback about this work?

What is the ETA for completing your to-do listed above? How can we help?

Hi @skoudoro , sorry for the long silence on my part. Last time we talked with Rafael, we agreed to remove the "regularization" from this implementation because we concluded it offers little benefit after the initialization for single-shell data, it was also becoming hard to make it compatible with the current multi-shell routine (point 2 of the TODO list).

Since then, I implemented the initialization routine (without reg) in a single function fernet_iter that takes single-voxel signals and is decorated with the multi_voxel_fit, and is therefore compatible with the models and classes of the current reconst.fwdti.py module; also modified the FreeWaterTensorModel class so that if it receives single-shell data, instead of aborting, it jumps to he single-shell routine. I did this in a separate branch https://github.com/mvgolub/dipy/blob/singleshell-fwdti-noreg/dipy/reconst/fwdti.py.

The docstrings are completed I think, so the only thing missing from the TODO list are the unit tests, which I am bad at designing them, so I will ask Rafael for some help. If you are familiar with the code in reconst.fwdti.py and have any suggestions on how we should approach the unit testing for the function that I added, feel free to share.

EDIT: the function fernet_iter was too long and confusing, I decided to separate it into three functions instead: fwmd_iter, fws0_iter and fwhy_iter, each corresponding to a specific free water initialization method.

@skoudoro
Copy link
Member

Thank you for the feedback and the update @mvgolub.

I will look into it and try to help also.

Currently, you have a small typo (+= instead of =+), that's why the tests are failing:

if not enough_b and self.fit_method in (wls_iter, nls_iter):
            mes = "fwDTI requires at least 3 b-values (which can include b=0)"
>           mes =+ " when using the NLS or WLS fit methods"
E           TypeError: bad operand type for unary +: 'str'

dipy/reconst/fwdti.py Outdated Show resolved Hide resolved
@arokem
Copy link
Contributor

arokem commented Jun 18, 2021

Fixed that typo in 7748b36

@codecov
Copy link

codecov bot commented Jun 18, 2021

Codecov Report

Merging #2087 (7748b36) into master (f74e47e) will decrease coverage by 6.54%.
The diff coverage is 6.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2087      +/-   ##
==========================================
- Coverage   91.21%   84.66%   -6.55%     
==========================================
  Files         250      125     -125     
  Lines       31803    16706   -15097     
  Branches     3340     2691     -649     
==========================================
- Hits        29008    14144   -14864     
+ Misses       2074     1877     -197     
+ Partials      721      685      -36     
Impacted Files Coverage Δ
dipy/reconst/fwdti.py 60.77% <6.29%> (-33.48%) ⬇️
dipy/nn/model.py 27.77% <0.00%> (-63.14%) ⬇️
dipy/viz/__init__.py 55.00% <0.00%> (-35.91%) ⬇️
dipy/workflows/viz.py 60.95% <0.00%> (-16.47%) ⬇️
dipy/viz/panel.py 63.82% <0.00%> (-9.94%) ⬇️
dipy/utils/optpkg.py 69.56% <0.00%> (-8.70%) ⬇️
dipy/workflows/stats.py 84.80% <0.00%> (-7.13%) ⬇️
dipy/utils/deprecator.py 94.07% <0.00%> (-5.93%) ⬇️
dipy/reconst/mcsd.py 88.36% <0.00%> (-5.55%) ⬇️
dipy/viz/regtools.py 31.49% <0.00%> (-5.31%) ⬇️
... and 188 more

@txusser
Copy link

txusser commented Aug 26, 2023

Hi. What is the status of this request? Has it merged? The pull request seems to be open.

Thanks in advance,

@skoudoro
Copy link
Member

Hi @txusser,

it is not ready to be merged. See below what is missing:

  • Add unit tests
  • Correct PEP8 errors
  • multiple reviews round.

your review and feedback (theory or code) is welcomed! Please let us know if you have any question concerning theory or code.

@txusser
Copy link

txusser commented Aug 27, 2023

I will try to look into it. This is so close to merge and it is a very relevant addition imho, it would be a pity for it to keep hanging.

@alrodrig11
Copy link

Has this ever been implemented in DIPY? I have single shell data that I would like to analyze using the same method/software as my multishell data.

@mbergamino
Copy link

mbergamino commented Oct 11, 2023 via email

@alrodrig11
Copy link

alrodrig11 commented Oct 11, 2023 via email

@skoudoro
Copy link
Member

Hi @alrodrig11,

As explained above, this PR needs a bit of work.

it is not ready to be merged. See below what is missing:

  • Add unit tests
  • Correct PEP8 errors
  • multiple reviews round.

your review and feedback (theory or code) is welcomed! Please let us know if you have any question concerning theory or code. You can easily get this branch, run the model and give us your opinion if it should be in or not.

Thank you for your future feedback

@skoudoro skoudoro force-pushed the master branch 7 times, most recently from 1419292 to ca6268a Compare December 8, 2023 22:25
@skoudoro skoudoro force-pushed the master branch 3 times, most recently from 5935e1e to 765963e Compare January 24, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants