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

[FIX] Updating DIPY tracking params #911

Merged
merged 3 commits into from
Oct 18, 2022
Merged

Conversation

36000
Copy link
Collaborator

@36000 36000 commented Oct 11, 2022

I am still a little confused by what the parameters input to DIPY's tractography algorithm means in terms of units. For example, a max length of 250mm causes problems for many bundles in HBN, however that should be long enough for a typical brain? This was triggered by lowering of max length from 1000mm in recent PR.

@arokem
Copy link
Collaborator

arokem commented Oct 11, 2022

Since we are using DIPY's local tracking with fixed_step set to True (the default), we believe that the size of the steps is defined in mm (https://github.com/dipy/dipy/blob/a8432c26b42051abf0fc14695b0794e9099476f6/dipy/tracking/localtrack.pyx#L44). This means that max_length and min_length should be in units of numbers of steps. Meaning that with this PR in place, our users are providing these inputs in units of mm (which makes sense) and then we convert (via the code in this PR) to number of steps, which then gets passed to DIPY to do the right thing. Whew.

@arokem
Copy link
Collaborator

arokem commented Oct 12, 2022

One thought regarding this is that we could verify what we think is correct by running this version on some data and then looking at the distribution of streamline lengths measured in mm. We can get that by taking the point-by-point Euclidean distance between nodes in each streamline, after transferring the streamlines to the RASMM space, where things should be, well, in mm. Then, we can ask what is the maximal length of the streamlines in the whole-brain tractography and see where the distribution cuts off. A bit empirical, but might serve to validate our reasoning. WDYT?

@36000
Copy link
Collaborator Author

36000 commented Oct 12, 2022

I checked and the distance between individual points is always ~0.5mm, then with max length 500mm, this is the distribution (units are in step size of 0.5mm unfortunately, so divide by 2 to get mm):
test

@36000
Copy link
Collaborator Author

36000 commented Oct 12, 2022

And here is the distribution for those accepted into at least one bundle:
test

@36000
Copy link
Collaborator Author

36000 commented Oct 12, 2022

And here for another HBN subjects:
test

@36000
Copy link
Collaborator Author

36000 commented Oct 12, 2022

OK, I think when I was checking lengths earlier I was not using the fix. With the fix in this PR, the HBN work with the matlab default max length of 250 and min length of 50. Will push that change and let's see if the automatic tests agree.

@36000
Copy link
Collaborator Author

36000 commented Oct 12, 2022

I think part of the confusion here was I was looking at just a bad HBN subject. I think the current setup should work, though 🤞

@36000 36000 changed the title [WIP] understanding DIPY tracking params Updating DIPY tracking params Oct 12, 2022
@36000 36000 changed the title Updating DIPY tracking params [FIX] Updating DIPY tracking params Oct 12, 2022
@36000
Copy link
Collaborator Author

36000 commented Oct 12, 2022

@arokem this is ready for review / merge.

@arokem
Copy link
Collaborator

arokem commented Oct 12, 2022

I am not sure that I understand what the conclusion of the testing you did was. Does the distribution of streamline lengths curtail at 500 steps (250mm)?

@36000
Copy link
Collaborator Author

36000 commented Oct 13, 2022

Yes, it looks like there are very few streamlines longer than 500 steps / 250 mm

@36000 36000 merged commit 101db5b into yeatmanlab:master Oct 18, 2022
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.

2 participants