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

dwi2mask overhaul (replacement PR) #2197

Merged
merged 107 commits into from
Feb 9, 2021
Merged

dwi2mask overhaul (replacement PR) #2197

merged 107 commits into from
Feb 9, 2021

Conversation

Lestropie
Copy link
Member

@Lestropie Lestropie commented Oct 12, 2020

#2088 is broken due to me wiping my fork (completely forgetting that that's where the dwi2mask updates were...) and GitHub refusing to re-establish the PR link; so relisting.

Given that the other OHBM Hackathon MRtrix3 project has matured to a preview release and should become a part of 3.1.0, I wanted to clear this off of my desk; there's also been a couple of private discussion threads where having this finalised would be beneficial. Listing as a full PR rather than a draft in the hope of getting some fresh eyes & opinions on it, but I will probably try to make some test data before merging.

There's some brain masking approaches that I tried to get working as a dwi2mask algorithm but couldn't get anything reasonable out of them, I still have the code if anyone wants to try themselves:
https://github.com/GUR9000/Deep_MRI_brain_extraction
https://pypi.org/project/deepbrain/
I'm open to implementing further algorithms to wrap existing approaches, but such approaches need to be compatible with T2-weighted images, and many newer brain extraction approaches are T1w tailored.

Because it's coming from my fork I don't have things set up to preview the documentation, but you can see the relevant raw pages:
https://github.com/Lestropie/mrtrix3/blob/dwi2mask/docs/dwi_preprocessing/masking.rst
https://github.com/Lestropie/mrtrix3/blob/dwi2mask/docs/reference/commands/dwi2mask.rst

@jdtournier: I thought about having hyperlinks in the dwi2mask help pages to the new dedicated docs page on the topic. But this would require parallel gymnastics to those used for getting version-matched hyperlinks into the SH-related binaries prior to tagging. Want to do it anyway?
Edit: If doing so, should also add links to the dwi2response docs page in the dwi2response help pages.

Tagging @wtsyeda and @RicardoRios46 so that they're aware of the progress given their contributions.

wtsyeda and others added 30 commits June 16, 2020 20:14
Deleted C++ verison of dwi2mask
Implemented python wrapper for dwi2mask
Implemented first dwi2mask algorithm fslbet
- dwi2mask hdbet: Utilises the "HD-BET" tool.
- dwi2mask legacy: Python-based reproduction of MRtrix3 dwi2mask binary command.
- dwi2mask template: Brain masking based on registration to a template image then transformation of a brain mask in template space back to the subject.
- dwi2mask ants: New stand-alone algorithm that invokes antsBrainExtraction.sh (as was done in the previous version of dwi2mask template).
- dwi2mask template with -software ants dow no longer invokes antsBrainExtraction.sh, and instead does a basic registration to template & inverse transformation of brain mask data.
[ENH] dwi2mask template: Enhancements
@Lestropie
Copy link
Member Author

Now includes content of Lestropie#2.

Would like to merge and encourage more widespread evaluation prior to 3.1.0.

@Lestropie
Copy link
Member Author

One other thing to confirm that nobody's unhappy with here is the naming of the additional maskfilter operation "bigblob". This integrates [ largest connected component -> inversion -> largest connected component -> inversion ] into a single operation, since it's a common chain. But open to suggestions for a better name.

@jdtournier
Copy link
Member

One other thing to confirm that nobody's unhappy with here is the naming of the additional maskfilter operation "bigblob". This integrates [ largest connected component -> inversion -> largest connected component -> inversion ] into a single operation, since it's a common chain. But open to suggestions for a better name.

Not sold on the 'bigblob' name - though it does have a certain appeal.

Just a thought, but given its mode of operation and the intention behind it, might it not be more appropriate to make that available as an option within the connected component filter, simply by adding -fill to the options alongside -largest...? Just an idea, but I reckon a command like:

maskfilter in.mif connect -largest -fill out.mif

would make sense?

@Lestropie
Copy link
Member Author

... as an option within the connected component filter, simply by adding -fill to the options alongside -largest

As long as -fill additionally applies to the connect filter without -largest, I think that works better. -largest is already slightly awkward in that it changes the nature of the output image from integer to bitwise; if -fill would then additionally be only applicable in instances where -largest is already used. you'd end up with two degrees of separation from the default filter operation. But if -fill and -largest are effectively independent, I'm good with that.

Just need to think about what to do if the filling of one component results in inclusion of voxels that are already in another component (e.g. concentric shells), since that result can't be stored as an integer label image.

@Lestropie
Copy link
Member Author

So it turns out there's more gymnastics involved in adding a fill operation to the connected component algorithm than foreseen, the implementation code ends up branching between fill and not moreso than the overlap if wanting to support filling in the absence of -largest.

So I think it in fact makes more sense to remove maskfilter bigblob and instead have a new filter fill, and just have various dwi2mask algorithms use ... - | maskfilter connect -largest - | maskfilter - fill - | .... If somebody wants to spatially fill multiple connected components, that can be done in a loop with maskfilter connect; mrcalc $INDEX -eq - | maskfilter - fill - | ..., and then they can themselves choose to not deal with the prospect of resulting overlaps.

@Lestropie Lestropie mentioned this pull request Feb 7, 2021
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.

5 participants