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

Local Tracking - BinaryTissueClassifier - dropping the last point #1661

Closed
jchoude opened this issue Nov 1, 2018 · 4 comments

Comments

@jchoude
Copy link
Contributor

commented Nov 1, 2018

Description

@gabknight This is the old classical question: should the last point of a streamline, the point that makes it finished (eg: reaching a 0 value in a Binary map), be kept or not in the tractogram?

When using local_tracking with a BinaryTissueClassifier, once a streamline exits the True valued voxels of the mask, a ENDPOINT flag is raised, the number of steps (i in the loop) is incremented by 1. This means that the tracker will keep the last point, even if, in fact, it is outside the mask.

I think we should probably drop that last point. Else, semantically, it doesn't make a lot of sense. Let's say I track, and then just do a endpoint filtering with the same mask, I will be missing some streamlines, since the endpoints will be outside of the mask.

@gabknight do you agree that this should be the behavior? If so, @czotti could help implementing it.

@gabknight

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

@jchoude yes ok. Removing the i += 1 in dipy/tracking/local/localtrack.pyx:189 should do the trick. Let's also remove the last point for the INVALIDPOINT case.

I would do that for local_tracker but not for pft_tracker. (BinaryTissueClassifier cannot be used for PFT, only ConstrainedTissueClassifier). Removing the last point of streamlines stopping in a voxel with lower then 100% GM is not ideal. Moreover, I would rather have all streamlines extend further in the GM for connectivity analysis.

The annoying thing with this is that PFTracking and localTracking will have different behavior when both use e.g. CMCTissueClassifier. I suggest clarifying what happen to the last points in the doc. What do you think?

@czotti Do you want to give it a go? The code modification is easy, but the modification will break all localTracking tests. :)

@czotti

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Sure I'll do it.

@jchoude

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

@gabknight Thanks for that! I agree that PFTracking shouldn't be modified.

@skoudoro

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

fixed by #1666. Closing this issue

@skoudoro skoudoro closed this Feb 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.