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

NF - Deterministic Maximum Direction Getter #495

Merged
merged 6 commits into from Dec 22, 2014

Conversation

Projects
None yet
6 participants
@gabknight
Contributor

gabknight commented Dec 10, 2014

New direction getter similar to the deterministic tracking of mrtrix.

  • do not follow peaks, but the maximum value of the pmf
  • i.e. probabilistic direction getter returning the maximum value of the distribution instead of drawing from it
  • pretty easy to add with this design @MrBago!

*whoop with the commit, I forgot one commit, so the RF - ranamed.. should be NF - new direction...

I want to rename the file "dipy/direction/probabilistic_direction_getter.py" to "dipy/direction/direction_getters.py". Should I do it here, or in another PR?

"""
The fiber orientation distribution (FOD) of the CSD model estimates the
distribution of small fiber bundles within each voxel. This distribution
can be use for deterministic fiber tracking. As for probabilistic tracking,

This comment has been minimized.

@samuelstjean

samuelstjean Dec 10, 2014

Contributor

can be used

The fiber orientation distribution (FOD) of the CSD model estimates the
distribution of small fiber bundles within each voxel. This distribution
can be use for deterministic fiber tracking. As for probabilistic tracking,
there are many ways to provide the deterministic maximum direction getter

This comment has been minimized.

@samuelstjean

samuelstjean Dec 10, 2014

Contributor

maybe missing a word here?

distribution of small fiber bundles within each voxel. This distribution
can be use for deterministic fiber tracking. As for probabilistic tracking,
there are many ways to provide the deterministic maximum direction getter
those distributions. Here, the spherical harmonic represnetation of the FOD

This comment has been minimized.

@samuelstjean

samuelstjean Dec 10, 2014

Contributor

typo representation

Deterministic maximum fiber tracking is an alternative to EuDX deterministic
tractography. Unlike EuDx, which follows the peaks of the local models,
deterministic maximum fiber tracking follows the trajectory of the most
probable pathway with the tracking constraint (e.g. max angle). It follows the

This comment has been minimized.

@samuelstjean

samuelstjean Dec 10, 2014

Contributor

with -> within

@gabknight

This comment has been minimized.

Contributor

gabknight commented Dec 10, 2014

thx @samuelstjean for the comments.

@@ -1 +1,2 @@
from .probabilistic_direction_getter import ProbabilisticDirectionGetter
from .probabilistic_direction_getter import (ProbabilisticDirectionGetter,
DeterministicMaximumDirectionGetter)

This comment has been minimized.

@arokem

arokem Dec 10, 2014

Member

Weird whitespace here

@arokem

This comment has been minimized.

Member

arokem commented Dec 12, 2014

OK - I've taken a more thorough look. The module code all looks good to me - thanks for doing this. One comment is that the example might confuse people. I would make it a bit clearer what the difference is between this and EuDX. In particular, this is "Deterministic tracking with CSD", I think, rather than the current title (which is accurate, but doesn't really help a complete beginner. In general, consider that users will come to this not knowing anything...). And it's worth saying in the example that this is an analogue of the deterministic tracking that mrtrix does.

The fiber orientation distribution (FOD) of the CSD model estimates the
distribution of small fiber bundles within each voxel. This distribution
can be used for deterministic fiber tracking. As for prob abilistic tracking,
there are many ways to provide those distributions tothe deterministic maximum

This comment has been minimized.

@arokem

arokem Dec 16, 2014

Member

typo: "tothe" => "to the"

"""
The fiber orientation distribution (FOD) of the CSD model estimates the
distribution of small fiber bundles within each voxel. This distribution
can be used for deterministic fiber tracking. As for prob abilistic tracking,

This comment has been minimized.

@arokem

arokem Dec 16, 2014

Member

typo: "prob abilistic" => "probabilistic"

@MrBago

This comment has been minimized.

Contributor

MrBago commented Dec 17, 2014

This looks really good @gabknight. I agree with @arokem, it would be good to highlight the difference between this direction getter and the peaks direction getter (EuDX).

Other than that I think this is almost ready to merge.

@gabknight gabknight force-pushed the gabknight:nf_add_maximum_det_dg branch from 774cea8 to 464deca Dec 22, 2014

@gabknight

This comment has been minimized.

Contributor

gabknight commented Dec 22, 2014

Sorry this should have been done much faster. Let me know what you think about the example update.

I also changed the gfa-based tissue classifier to an fa-based tissue classifier in this example. Should the fa be used as well in other tracking examples? I thought it was the case, but it isn't.

@mdesco

This comment has been minimized.

Contributor

mdesco commented Dec 22, 2014

On Mon, Dec 22, 2014 at 12:54 AM, Gabriel Girard notifications@github.com
wrote:

Sorry this should have been done much faster. Let me know what you think
about the example update.

I also changed the gfa-based tissue classifier to an fa-based tissue
classifier in this example. Should the fa be used as well in other tracking
examples? I thought it was the case, but it isn't.

Yes. I think so. Use FA instead of GFA.


Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/495#issuecomment-67804593.[image: Web
Bug from
https://github.com/notifications/beacon/ACEgB3s-rKg6Gq8UfarW0IlNxao5m7qVks5nZ6mxgaJpZM4DGw-Y.gif]

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Dec 22, 2014

Thank you @gabknight .There are only some minor pep8 corrections. Don't worry about those. I can fix them in upcoming PR and link your tutorial to the new index. Looks good in general. If you can also go forward with ACT PR. Do it. We are realizing on the 26th-27th. Let me know if you need any help with the datasets.

Garyfallidis added a commit that referenced this pull request Dec 22, 2014

Merge pull request #495 from gabknight/nf_add_maximum_det_dg
NF - Deterministic Maximum Direction Getter

@Garyfallidis Garyfallidis merged commit 86e3477 into nipy:master Dec 22, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@gabknight gabknight deleted the gabknight:nf_add_maximum_det_dg branch Jun 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment