-
Notifications
You must be signed in to change notification settings - Fork 40
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: Use t1w2fsnative_xfm to resample segmentations #201
Conversation
0f581fd
to
780f875
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great.
Maybe it's going to be easier to change the base to maint/20.0.x to avoid the license problem. Then progress changes to 20.1 and master (and backport if we get on a commitment strike).
Okay it was late when I reviewed this and I was probably unclear on my suggestion. What I meant with the PR base is:
|
780f875
to
b56c615
Compare
Diff: diff --git a/.circleci/config.yml b/.circleci/config.yml
index ba2167709..09d304ca7 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -249,7 +249,8 @@ jobs:
pyenv local $PY2
echo -n "Python version: "
python --version
- pip install --upgrade pip setuptools
+ pip install --upgrade "pip<21"
+ pip install --upgrade setuptools
pip install --upgrade wrapper/
which smriprep-docker
smriprep-docker -i poldracklab/smriprep:latest --help
diff --git a/smriprep/workflows/surfaces.py b/smriprep/workflows/surfaces.py
index 9e6702323..cd77b1536 100644
--- a/smriprep/workflows/surfaces.py
+++ b/smriprep/workflows/surfaces.py
@@ -231,10 +231,12 @@ gray-matter of Mindboggle [RRID:SCR_002438, @mindboggle].
(autorecon_resume_wf, aseg_to_native_wf, [
('outputnode.subjects_dir', 'inputnode.subjects_dir'),
('outputnode.subject_id', 'inputnode.subject_id')]),
+ (fsnative2t1w_xfm, aseg_to_native_wf, [('out_reg_file', 'inputnode.fsnative2t1w_xfm')]),
(inputnode, aparc_to_native_wf, [('corrected_t1', 'inputnode.in_file')]),
(autorecon_resume_wf, aparc_to_native_wf, [
('outputnode.subjects_dir', 'inputnode.subjects_dir'),
('outputnode.subject_id', 'inputnode.subject_id')]),
+ (fsnative2t1w_xfm, aparc_to_native_wf, [('out_reg_file', 'inputnode.fsnative2t1w_xfm')]),
(aseg_to_native_wf, refine, [('outputnode.out_file', 'in_aseg')]),
# Output
@@ -502,6 +504,8 @@ def init_segs_to_native_wf(name='segs_to_native', segmentation='aseg'):
FreeSurfer SUBJECTS_DIR
subject_id
FreeSurfer subject ID
+ fsnative2t1w_xfm
+ LTA-style affine matrix translating from FreeSurfer-conformed subject space to T1w
Outputs
-------
@@ -510,13 +514,15 @@ def init_segs_to_native_wf(name='segs_to_native', segmentation='aseg'):
"""
workflow = Workflow(name='%s_%s' % (name, segmentation))
- inputnode = pe.Node(niu.IdentityInterface([
- 'in_file', 'subjects_dir', 'subject_id']), name='inputnode')
+ inputnode = pe.Node(
+ niu.IdentityInterface(['in_file', 'subjects_dir', 'subject_id', 'fsnative2t1w_xfm']),
+ name='inputnode')
outputnode = pe.Node(niu.IdentityInterface(['out_file']), name='outputnode')
# Extract the aseg and aparc+aseg outputs
fssource = pe.Node(nio.FreeSurferSource(), name='fs_datasource')
- tonative = pe.Node(fs.Label2Vol(), name='tonative')
- tonii = pe.Node(fs.MRIConvert(out_type='niigz', resample_type='nearest'), name='tonii')
+ # Resample from T1.mgz to T1w.nii.gz, applying any offset in fsnative2t1w_xfm,
+ # and convert to NIfTI while we're at it
+ resample = pe.Node(fs.ApplyVolTransform(transformed_file='seg.nii.gz'), name='resample')
if segmentation.startswith('aparc'):
if segmentation == 'aparc_aseg':
@@ -531,12 +537,10 @@ def init_segs_to_native_wf(name='segs_to_native', segmentation='aseg'):
(inputnode, fssource, [
('subjects_dir', 'subjects_dir'),
('subject_id', 'subject_id')]),
- (inputnode, tonii, [('in_file', 'reslice_like')]),
- (fssource, tonative, [(segmentation, 'seg_file'),
- ('rawavg', 'template_file'),
- ('aseg', 'reg_header')]),
- (tonative, tonii, [('vol_label_file', 'in_file')]),
- (tonii, outputnode, [('out_file', 'out_file')]),
+ (inputnode, resample, [('in_file', 'target_file'),
+ ('fsnative2t1w_xfm', 'lta_file')]),
+ (fssource, resample, [(segmentation, 'source_file')]),
+ (resample, outputnode, [('transformed_file', 'out_file')]),
])
return workflow |
No. |
Closes #154.
As noted in #154, if fsnative is not pre-aligned with the T1w template fMRIPrep produces, aparc/aseg do not get properly aligned, and due to using FreeSurfer outputs to refine the brainmask, everything suffers.
This first pipes the
t1w2fsnative_xfm
to the segmentation workflows (init_segs_to_native_wf
). This workflow had usedmri_label2vol
+mri_convert
. Themri_label2vol
was already converting to NIfTI, so I droppedmri_convert
.I manually confirmed that using
mri_label2vol
with the inverse LTA gets the alignment right.I also noticed that
mri_label2vol
has label, annotation and volume modes. Sinceaseg.mgz
andaparc+aseg.mgz
are already volumes (.mgz
, rather than.label
or.annot
), I checked whethermri_vol2vol
could be used.That also worked. Even better, it takes its transforms in the usual direction (source -> target), so we don't have to leave comments about why we either pass the inverted LTA file, or pass the normal file and an "invert" flag.
Because the interface changed, I changed the name of the node.
Bug-fix status: This changes the workflow, but it does so in a way that should keep working directories reusable. And it fixes a bug in several versions of fMRIPrep. So I would suggest that we make an exception and release this as a bug-fix, which will allow us to fix the 20.0 and 20.1 series (and backport to 1.5, if it ever comes up).