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
Merged

Conversation

arokem
Copy link
Contributor

@arokem arokem 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!

Copy link
Member

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor Author

arokem 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
Copy link
Contributor Author

arokem 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
Copy link
Contributor

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
Copy link

codecov-io 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
Copy link

pep8speaks 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

dipy/tracking/streamline.py Outdated Show resolved Hide resolved
@arokem
Copy link
Contributor Author

arokem 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.

Copy link
Member

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[node::n_points] Nice! 👍

@skoudoro
Copy link
Member

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

@arokem
Copy link
Contributor Author

arokem commented Jan 14, 2019 via email

@arokem
Copy link
Contributor Author

arokem commented Jan 22, 2019

Any more thoughts here?

@arokem
Copy link
Contributor Author

arokem commented Jan 26, 2019

Rebased again, after the #1652 merge.

Copy link
Member

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase residue

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase residue

@arokem
Copy link
Contributor Author

arokem commented Feb 1, 2019 via email

@arokem
Copy link
Contributor Author

arokem commented Feb 12, 2019

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

@skoudoro
Copy link
Member

ping @Garyfallidis

@arokem
Copy link
Contributor Author

arokem commented Feb 13, 2019

Looks like a new release of cvxpy is breaking MAPMRI.

@skoudoro
Copy link
Member

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
Copy link
Member

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

@arokem
Copy link
Contributor Author

arokem commented Feb 14, 2019

Pin the cvxpy version to the previous release for now?

@arokem
Copy link
Contributor Author

arokem commented Feb 14, 2019

Implemented in cbf9275, I think.

@skoudoro
Copy link
Member

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

@arokem
Copy link
Contributor Author

arokem commented Feb 27, 2019

Now also rebased.

@Garyfallidis
Copy link
Contributor

@arokem ready for final check?

@arokem
Copy link
Contributor Author

arokem commented Feb 27, 2019

I believe so!

return w / np.sum(w, 0)


def afq_tract_profile(data, bundle, affine=None, n_points=100,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Garyfallidis
Copy link
Contributor

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

@Garyfallidis
Copy link
Contributor

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
Copy link
Contributor Author

arokem commented Feb 27, 2019

Yeah - good call. Renamed!

@Garyfallidis Garyfallidis merged commit 666b706 into dipy:master Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants