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

A few tractometry functions #1695

Merged
merged 33 commits into from Mar 1, 2019

Conversation

@arokem
Copy link
Member

commented Dec 17, 2018

For tractometry, we want to be able to extract the profiles along the length of a bundle

This PR introduces a couple of useful functions for that:

  • bundle_profile takes a bundle and returns the values along the length of the bundle, resampled to 100 (or some other number of) nodes.
  • gaussian_weights allows you to weight the streamlines within a bundle, depending on how far they are from the core of the bundle.
  • Finally, orient_by_streamline will make sure that a bundle is all oriented in the same direction (using the other functions without doing this will result in erroneous results!), based on the trajectory of a "standard" streamline.

Thanks to @jyeatman for guidance in implementing these in the prototype implementation we are using in pyAFQ!

@skoudoro
Copy link
Member

left a comment

Thank you for this @arokem. I just have a few request:

  • Can you rebase this PR?
  • Can you add a short tutorial here?
  • I think it will be better to avoid list of array promotion and use Streamlines class everywhere. I do not understand why you convert your Streamlines object into a np.array. It should be the opposite.
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
Parameters
----------
data : 3D volume
bundle : StreamLines class instance, list of arrays, or array

This comment has been minimized.

Copy link
@skoudoro

skoudoro Jan 3, 2019

Member

due to memory issue, I think we should avoid to promote list of arrays and only use Streamlines class instance.

This comment has been minimized.

Copy link
@arokem

arokem Jan 6, 2019

Author Member

OK. Changed here and in the docstring above.

dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
@arokem

This comment has been minimized.

Copy link
Member Author

commented Jan 4, 2019

I am trying to create an example, which would follow the bundle extraction example, using these bundles to do tractometry. For that, I will also need parameter maps for this subject.

@Garyfallidis: Which of the HCP subjects was used to create the fetch_target_tractogram_hcp tractogram?

@arokem

This comment has been minimized.

Copy link
Member Author

commented Jan 4, 2019

Sorry - @BramshQamar : I see that you made that tutorial. Do you know which subject fetch_target_tractogram_hcp belongs to?

@BramshQamar

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2019

Sorry - @BramshQamar : I see that you made that tutorial. Do you know which subject fetch_target_tractogram_hcp belongs to?

Hello @arokem, we used subject 100206 from HCP data to create target tractogram for bundle extraction example.

@codecov-io

This comment has been minimized.

Copy link

commented Jan 5, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@61dbe1d). Click here to learn what that means.
The diff coverage is 95.94%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1695   +/-   ##
=========================================
  Coverage          ?   84.29%           
=========================================
  Files             ?      115           
  Lines             ?    13748           
  Branches          ?     2176           
=========================================
  Hits              ?    11589           
  Misses            ?     1653           
  Partials          ?      506
Impacted Files Coverage Δ
dipy/data/__init__.py 82.05% <ø> (ø)
dipy/segment/bundles.py 91.8% <100%> (ø)
dipy/data/fetcher.py 33.59% <100%> (ø)
dipy/align/streamlinear.py 87.17% <100%> (ø)
dipy/tracking/streamline.py 70.73% <91.17%> (ø)
@pep8speaks

This comment has been minimized.

Copy link

commented Jan 5, 2019

Hello @arokem, Thank you for updating !

Line 453:9: E128 continuation line under-indented for visual indent

Line 723:16: E222 multiple spaces after operator

Line 338:37: E128 continuation line under-indented for visual indent
Line 339:37: E128 continuation line under-indented for visual indent

Line 43:1: E402 module level import not at top of file
Line 63:1: E402 module level import not at top of file
Line 64:1: E402 module level import not at top of file
Line 71:1: E402 module level import not at top of file
Line 73:1: E402 module level import not at top of file
Line 100:1: E402 module level import not at top of file
Line 111:81: E501 line too long (84 > 80 characters)
Line 117:1: E402 module level import not at top of file
Line 126:1: E402 module level import not at top of file
Line 173:4: W292 no newline at end of file

Line 132:42: E231 missing whitespace after ','
Line 132:45: E231 missing whitespace after ','
Line 133:47: E231 missing whitespace after ','
Line 133:50: E231 missing whitespace after ','
Line 157:10: E201 whitespace after '('
Line 178:43: E231 missing whitespace after ','
Line 178:46: E231 missing whitespace after ','
Line 179:48: E231 missing whitespace after ','
Line 179:51: E231 missing whitespace after ','
Line 215:4: W292 no newline at end of file

Comment last updated on February 28, 2019 at 03:44 Hours UTC

@arokem arokem force-pushed the arokem:bundle_profiles branch from 07cfef0 to b938a61 Jan 6, 2019

dipy/tracking/streamline.py Outdated Show resolved Hide resolved
@arokem

This comment has been minimized.

Copy link
Member Author

commented Jan 6, 2019

I added an example usage in f727bde. I had to introduce some changes in the bundle extraction example, because I am reusing the bundles generated in that example.

@skoudoro
Copy link
Member

left a comment

Thank you for the tutorial! I need to play a bit more with it but in overall, It looks good to me.

dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
dipy/tracking/streamline.py Outdated Show resolved Hide resolved
doc/examples/bundle_extraction.py Outdated Show resolved Hide resolved
doc/examples/bundle_extraction.py Outdated Show resolved Hide resolved
# This should come back as a 3D covariance matrix with the spatial
# variance covariance of this node across the different streamlines
# This is a 3-by-3 array:
node_coords = bundle.data[node::n_points]

This comment has been minimized.

Copy link
@skoudoro

skoudoro Jan 8, 2019

Member

[node::n_points] Nice! 👍

@skoudoro

This comment has been minimized.

Copy link
Member

commented Jan 14, 2019

After a rebase, this PR is ready to go, it will be good if someone could have a look on this one

@arokem arokem force-pushed the arokem:bundle_profiles branch from 9577808 to 7fff8bd Jan 14, 2019

@arokem

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

@arokem

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2019

Any more thoughts here?

@arokem arokem force-pushed the arokem:bundle_profiles branch from 7dcbd28 to 011797a Jan 26, 2019

@arokem

This comment has been minimized.

Copy link
Member Author

commented Jan 26, 2019

Rebased again, after the #1652 merge.

@skoudoro
Copy link
Member

left a comment

I will merge this on Wednesday if there is no other comment.

Can you fix the small rebase residue below @arokem?

@@ -167,6 +167,10 @@ Streamline analysis and connectivity
- :ref:`example_streamline_length`
- :ref:`example_cluster_confidence`
- :ref:`example_path_length_map`
<<<<<<< HEAD

This comment has been minimized.

Copy link
@skoudoro

skoudoro Feb 1, 2019

Member

rebase residue

<<<<<<< HEAD
=======
- :ref:`example_bundle_profiles`
>>>>>>> Adds an example of bundle profile extraction (tractometry)

This comment has been minimized.

Copy link
@skoudoro

skoudoro Feb 1, 2019

Member

rebase residue

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 1, 2019

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 12, 2019

Does anyone else want to take a look? Any reason not to merge this?

@skoudoro

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

@arokem arokem force-pushed the arokem:bundle_profiles branch from 867ea80 to 8e95524 Feb 13, 2019

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 13, 2019

Looks like a new release of cvxpy is breaking MAPMRI.

@skoudoro

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

I just did a quick check on cvxpy changelog and there are very few changes. Mapmri tests seems really sensitive... I will have a deeper look later

@skoudoro

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

Hi @arokem, I just created this issue cvxgrp/cvxpy#672 for our cvxpy problem. Concerning Travis, I do not know what should be our strategy to make Travis happy until their fix

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2019

Pin the cvxpy version to the previous release for now?

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2019

Implemented in cbf9275, I think.

@skoudoro

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

Great! Thanks! we have to put a reminder somewhere for coming back to normal later

@arokem arokem force-pushed the arokem:bundle_profiles branch from cbf9275 to 61271d9 Feb 15, 2019

@arokem arokem force-pushed the arokem:bundle_profiles branch from f876c9e to 4ca465a Feb 27, 2019

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

Now also rebased.

arokem added 2 commits Feb 27, 2019
@Garyfallidis

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

@arokem ready for final check?

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

I believe so!

return w / np.sum(w, 0)


def afq_tract_profile(data, bundle, affine=None, n_points=100,

This comment has been minimized.

Copy link
@Garyfallidis

Garyfallidis Feb 27, 2019

Member

You are already in bundles.py no need for tract. I would just say afq_profile.

@Garyfallidis

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

I added a last comment. Please correct and ping me to merge! :)

@Garyfallidis

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

In general I use bundles to mean tracts, fascicles, muscle fibers etc. The tools are quite generic so I think bundles is good name to use. I don't think we need to specify tracts anymore. However, in the tutorial it is good to have some anatomical naming standards. Which you do 👍

@arokem

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

Yeah - good call. Renamed!

@arokem arokem force-pushed the arokem:bundle_profiles branch from 616abbe to f898e1a Feb 28, 2019

@Garyfallidis Garyfallidis merged commit 666b706 into nipy:master Mar 1, 2019

4 checks passed

codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.