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

tckgen/tckedit: warn user if ROI voxel size is smaller than step size of streamlines #1855

Closed
jdtournier opened this issue Dec 18, 2019 · 6 comments
Assignees
Labels

Comments

@jdtournier
Copy link
Member

@jdtournier jdtournier commented Dec 18, 2019

As discussed on forum.

Would like a more fool-proof strategy going forward, but I can't think of a simple solution that won't incur some performance penalty...

@jdtournier jdtournier added this to the MRtrix3 3.0 release milestone Dec 18, 2019
@Lestropie

This comment has been minimized.

Copy link
Member

@Lestropie Lestropie commented Dec 19, 2019

"Simple solution" would be to add a -precise option to tckedit, which mirrors the one in tckmap by engaging this function and comparing the output SetVoxel to the mask image.

@jdtournier

This comment has been minimized.

Copy link
Member Author

@jdtournier jdtournier commented Dec 19, 2019

Sounds appealing. Two concerns though:

  • what's the performance penalty? I'm hoping minimal given that most of the work should be in the ODF sampler (at least for iFOD1/2)...

  • I can't remember whether we interpolate the ROIs during tracking - if so, that would introduce a divergence in behaviour. Not a concern if we don't - which I think is the case given this piece of code...?

@Lestropie

This comment has been minimized.

Copy link
Member

@Lestropie Lestropie commented Dec 19, 2019

I seem to recall having made an explicit decision at some point to not only not interpolate masks during tracking, but also to remove an old command-line option that toggled the behaviour.

what's the performance penalty?

For test runs of tckmap using -upsample 1 (to disable upsampling) for 10M streamlines to 1mm voxel grid, comparing default behaviour to -precise, runtime went from 55s to 2m20s.

Other factor to consider is that by using this mechanism, streamlines can only be comprehensively compared to all ROIs once the entire streamline trajectory has been produced. This is the same for ACT with back-tracking: if a streamline were to intersect an include region, only to then be truncated such that it then fails to intersect that region, the fact that the final streamline fails to intersect all include regions needs to result in its rejection; see code here and here.

@Lestropie

This comment has been minimized.

Copy link
Member

@Lestropie Lestropie commented Jan 1, 2020

Follow-up thought:

While adding a -precise option to tckedit would probably make reasonable sense, doing the same in tckgen would not be so intuitive; but the behaviour of those two commands w.r.t. ROIs really needs to be the same...

Given the comparably short computation time of this step relative to tracking, are we better off just using this mapping mechanism throughout all relevant commands always?

  • For tckgen & tckedit there's no need to perform any upsampling, that's only really necessary to get more accurate voxel intersection lengths.

  • tckmap could have a -legacy option to use the current default behaviour if desired.

  • As described above, tckgen could only robustly reject streamlines for entering an exclude region once the entire streamline has been generated. It could however still do the "basic" intersection tests during propagation and only perform the comprehensive test at completion if that were preferable.

@Lestropie

This comment has been minimized.

Copy link
Member

@Lestropie Lestropie commented Jan 2, 2020

Also appeared in a second forum thread.

@Lestropie

This comment has been minimized.

Copy link
Member

@Lestropie Lestropie commented Jan 17, 2020

Closed due to creation of #1882.

@Lestropie Lestropie closed this Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.