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] INU correction before merging several T1w #925

Merged
merged 9 commits into from Jan 10, 2018

Conversation

oesteban
Copy link
Member

@oesteban oesteban commented Jan 9, 2018

Otherwise, we get funky composite bias fields that N4 can't recover.
This will also make coregistration between T1s robuster.

Fixes #924

Otherwise, we get funky composite bias fields that N4 can't recover.
This will also make coregistration between T1s robuster.
@oesteban oesteban changed the title [FIX] INU correction before merging several T1w [FIX,WIP] INU correction before merging several T1w Jan 9, 2018
@oesteban oesteban changed the title [FIX,WIP] INU correction before merging several T1w [FIX] INU correction before merging several T1w Jan 9, 2018
@oesteban oesteban requested a review from effigies January 9, 2018 14:39
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable, though I think we can simplify further in the N = 1 case.

My main concern will be whether doubling up of N4 correction (because it's also performed in the skull-stripping step) might cause artifacts.

Possibly a simpler (conceptually, if not in practice) solution would be to factor N4 out of skull-stripping, apply before merging, and eliminate the need to branch on number of T1w images at all.


workflow.connect([
(t1_conform, t1_reorient, [(('out_file', _get_first), 'in_file')]),
(t1_reorient, concat_affines, [('transform', 'mat_BtoC')]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t1_reorient is not necessary if t1_merge is not used. Conformation orients to RAS, but for reasons I don't understand, merging sometimes changes the orientation. Then you can get rid of concat_affines and just use the conformation affine.

@oesteban
Copy link
Member Author

oesteban commented Jan 9, 2018

I've been working at times in a pure-nipype implementation of antsBrainExtraction.sh. However, I haven't got any decent prototype yet (lack of time).

I'm not particularly worried about running N4 twice though, as long as it gets us sensible brain masks. But I agree, it'd be better not doubling up.

@effigies
Copy link
Member

effigies commented Jan 9, 2018

Okay, so if we're fairly confident that the quality of the brain mask would be unaffected, but don't want to risk any artifacts of doubling up N4, we could use the following sequence in all cases:

  • Conform
  • N4-correct
  • Merge
  • Skull-strip
  • Apply brain mask to merged image

This would mean we stop using the BrainExtractionBrain and N4Corrected0 outputs of brain extraction directly. We could slide the extraction into the skullstrip_ants_wf to keep the interfaces consistent.

WDYT?

@oesteban
Copy link
Member Author

oesteban commented Jan 9, 2018

Yes, though I would give it a try to what we have right now.

My real, deep interest in deconstructing antsBrainExtraction.sh is to be able to connect stuff in the middle, without modifying the nipype interface, and to allow for tweaks. Great things we could have:

  • Find a mapping from OASIS (or NKI) to MNI and initialize spatial normalization with the precomputed transform of brain masking (remove 20 min from runtime).
  • Run the brain extraction on all T1s separately:
    1. Run an antsTemplate -like workflow to replace the fs.RobustTemplate.
    2. Initialize second and subsequent transforms with the first OASIS-T1w transform followed by the affine found in i).
    3. Final brainmask is the consensus of all masks.

But I can't see this happening anytime soon.

@effigies
Copy link
Member

effigies commented Jan 9, 2018

Alright. So we can keep this if branch for now, and maybe create an issue to move to avoid having double N4 correction in skull-stripping outputs. It shouldn't be a huge task, but it will still be some work.

@oesteban
Copy link
Member Author

oesteban commented Jan 9, 2018

I'd go for that. I'm simplifying the workflow following your suggestions.

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. Some small comments.

CHANGES.rst Outdated
@@ -1,6 +1,7 @@
1.0.3 (3rd of January 2018)
===========================

* [FIX] INU correction before merging several T1w (#925).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should go in a "Next release" section.

#Transform 0
Transform: MatrixOffsetTransformBase_double_3_3
Parameters: 1 0 0 0 1 0 0 0 1 0 0 0
FixedParameters: 0 0 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this file will need to be added to setup.py?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

concat_affines = pe.MapNode(
ConcatAffines(3, invert=True), iterfield=['mat_AtoB', 'mat_BtoC'],
ConcatAffines(1 + min(num_t1w, 2), invert=True),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can revert lta_to_fsl and concat_affines entirely, cleaning up the diff a little.

In any event, this no longer needs the 1 + min(...) construct.

@oesteban oesteban merged commit 682b507 into nipreps:master Jan 10, 2018
@oesteban oesteban deleted the fix/bias-before-t1_merge branch January 10, 2018 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants