[RTM] Replace FLIRT-BBR with FreeSurfer bbregister when FS preprocessing is enabled #362
Conversation
|
It would be important to make sure that the transformation matrix produced by bbregister moves the image to the original T1 space not to the FS conformed T1. |
effigies
force-pushed
the
bbregister
branch
2 times, most recently
from
c736fe9 to
cfbeee4
Compare
|
@oesteban Could you have a look at https://circleci.com/gh/effigies/fmriprep/102? This node is getting mis-connected, it looks like. This shouldn't have been touched by the changes here. But I suppose it's possible that it's something on the nipype end with your recent PRs that we haven't adjusted for? (I'm pinning a newer nipype.) |
|
BTW - I think you can switch to main branch on both niworkflows and nipype now. |
|
I have a niworkflows branch (no PR yet) with changes needed for this bit. When I'm sure I don't need more, I'll put a PR there. |
fmriprep/workflows/epi.py
Outdated
| contrast_type='t2', | ||
| init='header', | ||
| registered_file=True, | ||
| out_fsl_file=True, |
There was a problem hiding this comment.
Running BBRegisterRPT, I'm running into the issue that registered_file and out_reg_file are undefined in the outputs, but not in the command (which uses _list_outputs to build the command). Neither exists in the working directory. Weirdly, out_fsl_file is defined and exists, so it doesn't seem like it should be an mixin/MRO issue.
Node: sub-387 (ref_epi_t1_registration (bbregister (registration)
=================================================================
Hierarchy : workflow_enumerator.sub-387.ref_epi_t1_registration.bbregister
Exec ID : bbregister.a2
Original Inputs
---------------
* args : <undefined>
* compress_report : auto
* contrast_type : t2
* environ : {'SUBJECTS_DIR': '/opt/freesurfer/subjects'}
* epi_mask : <undefined>
* generate_report : True
* ignore_exception : False
* init : header
* init_reg_file : <undefined>
* intermediate_file : <undefined>
* out_fsl_file : True
* out_reg_file : <undefined>
* out_report : report.svg
* reg_frame : <undefined>
* reg_middle_frame : <undefined>
* registered_file : True
* source_file : /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/explicit_mask_epi/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked.nii.gz
* spm_nifti : <undefined>
* subject_id : sub-387
* subjects_dir : /out/freesurfer
* terminal_output : allatonce
Execution Inputs
----------------
* args : <undefined>
* compress_report : auto
* contrast_type : t2
* environ : {'SUBJECTS_DIR': '/out/freesurfer'}
* epi_mask : <undefined>
* generate_report : True
* ignore_exception : False
* init : header
* init_reg_file : <undefined>
* intermediate_file : <undefined>
* out_fsl_file : True
* out_reg_file : <undefined>
* out_report : report.svg
* reg_frame : <undefined>
* reg_middle_frame : <undefined>
* registered_file : True
* source_file : /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked.nii.gz
* spm_nifti : <undefined>
* subject_id : sub-387
* subjects_dir : /out/freesurfer
* terminal_output : allatonce
Execution Outputs
-----------------
* min_cost_file : <undefined>
* out_fsl_file : /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked_bbreg_sub-387.mat
* out_reg_file : <undefined>
* out_report : /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/report.svg
* registered_file : <undefined>
Runtime info
------------
* command : bbregister --t2 --init-header --fslmat /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked_bbreg_sub-387.mat --reg /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked_bbreg_sub-387.dat --o /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked_bbreg.nii.gz --mov /scratch/workflow_enumerator/sub-387/ref_epi_t1_registration/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked.nii.gz --s sub-387
* duration : 67.60915
* hostname : 9c7c0a3e6622
Terminal output
~~~~~~~~~~~~~~~@chrisfilo Anything obvious stick out to you?
There was a problem hiding this comment.
Oh. source_file changed.
Was: [...]/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/explicit_mask_epi/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked.nii.gz
Now: [...]/_epi_..data..sub-387..func..sub-387_task-Beast_run-3_bold.nii.gz/bbregister/sub-387_task-Beast_run-3_bold_mcf.nii.gz_mean_reg_masked.nii.gz
Looks like a nipype issue. I'll try to figure this out.
There was a problem hiding this comment.
Ugh. remove_unnecessary_outputs is True by default.
It does seem like if I say "Make this file", it should at least throw a warning if it's unused, but I suppose setting a config variable is easy enough.
There was a problem hiding this comment.
We should definitely make reportlets work independently off what remove_unnecessary_outputs is set to.
As far as I remember with DEBUG logging you get information which files are removed.
There was a problem hiding this comment.
Reportlets work fine (mainly because we hook them up to an output sink). It's the outputs like the registered file (which I wanted to visually compare to the FLIRT-registered file) that get dropped if there isn't something else grabbing them.
Unfortunately, needed_outputs is an attribute of Node, not BaseInterface, so there's no simple way for an interface to mark the outputs that it uses as do-not-erase.
And yeah, the DEBUG logging is how I realized they were at least being created.
|
Probably has something to do with |
|
Yeah, it did, and wasn't at all the problem. Well, I guess I understand more of nipype's internals now. 🥇 |
|
Okay. We're definitely getting different outputs, and it appears that Visually, the output images align. sub-387_T1w.nii.gzsub-387_task-Beast_run-1_bold.nii.gzsub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_flirt.nii.gzsub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_bbreg.nii.gzsub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_flirt.matsub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_bbreg_sub-387.mat |
|
It might be good to pick @satra brains on this or peruse the workflows in
nipype. I have a feeling this might have been dealt with in the past.
…On Thu, Feb 23, 2017 at 9:03 AM, Chris Markiewicz ***@***.***> wrote:
Okay. We're definitely getting different outputs, and it appears that
bbregister is registering to the 256x256x256 FreeSurfer space.
Visually, the output images align.
sub-387_T1w.nii.gz
Dimensions: 256 x 256 x 186
Voxel sizes: 0.8984 x 0.8984 x 0.9
Affine: [[ 0. 0. -0.89999998 78.08100128]
[ 0. -0.89840001 0. 131.02000427]
[ -0.89840001 0. 0. 110.31900024]
[ 0. 0. 0. 1. ]]
sub-387_task-Beast_run-1_bold.nii.gz
Dimensions: 100 x 100 x 64 x 650
Voxel sizes: 2.2 x 2.2 x 2.2 x 0.68
Affine: [[ -2.20000005e+00 0.00000000e+00 1.99987669e-03 1.00387001e+02]
[ 7.35627196e-04 -1.97774065e+00 9.63940501e-01 8.25609970e+01]
[ -1.50989520e-03 -9.63606596e-01 -1.97757792e+00 1.03964500e+02]
[ 0.00000000e+00 0.00000000e+00 0.00000000e+00 1.00000000e+00]]
sub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_flirt.nii.gz
Dimensions: 186 x 256 x 256
Voxel sizes: 0.9 x 0.8984 x 0.8984
Affine: [[ 0.89999998 0. 0. -88.41899109]
[ 0. 0.89840001 0. -98.0719986 ]
[ 0. 0. 0.89840001 -118.77300262]
[ 0. 0. 0. 1. ]]
sub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_bbreg.nii.gz
Dimensions: 256 x 256 x 256
Voxel sizes: 0.8984 x 0.8984 x 0.8984
Affine: [[ -0.89840013 0. 0. 109.3762207 ]
[ 0. 0. 0.89840001 -98.97039795]
[ 0. -0.89840001 0. 110.31900024]
[ 0. 0. 0. 1. ]]
sub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_flirt.mat
Registration: [[ 9.99822613e-01 2.18406143e-03 -1.87130549e-02 -2.01251828e+01]
[ 1.02471905e-02 -8.96531511e-01 4.42861559e-01 1.78893223e+02]
[ -1.58096043e-02 -4.42974773e-01 -8.96394706e-01 2.26519183e+02]
[ 0.00000000e+00 0.00000000e+00 0.00000000e+00 1.00000000e+00]]
sub-387_task-Beast_run-1_bold_mcf.nii.gz_mean_reg_masked_bbreg_sub-387.mat
Registration: [[ 9.99783580e-01 4.73640000e-04 -2.07878600e-02 1.14698811e+01]
[ 1.83094900e-02 4.41616650e-01 8.96942620e-01 2.37868500e+00]
[ 9.54438000e-03 -8.97202850e-01 4.41665860e-01 1.80107742e+02]
[ 0.00000000e+00 0.00000000e+00 0.00000000e+00 1.00000012e+00]]
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#362 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAOkpyZnJiL8x0ymUkh1q-M48G5ZF__tks5rfbvSgaJpZM4MDxd7>
.
|
effigies
force-pushed
the
bbregister
branch
3 times, most recently
from
ecb18f0 to
584367e
Compare
|
So 7e56d04 implements the FreeSurfer-conformed volume -> original T1w volume transformation. However, now that I'm trying to think through it, I'm confusing myself. Basic idea: @satra, if you get a second, could you have a look and let me know if this is what you were describing? If anybody else sees something obvious, let me know. |
fsl1.mat -> reference is regular T1.nii so technically if you know the mapping between T1.nii and freesurfer space, you should be able to go one way or the other. of the top of my head i was thinking that this would be the same as the mapping between fsl1.mat and fsl2.mat i.e., pinv(fsl1.mat).dot(fsl2.mat) or vice verse depending on which transform you want. t1w_affine maps from ijk_t1w to xyz it's the xyz that's common to both, which is what allows you to relate them. |
|
I think what you describe -- mapping between the two structural spaces using the functional registrations -- assumes that FSL BBR and FS bbregister are going to produce the same alignment? If that were the case, we should just choose the one that puts us in the space we want, in this case FSL. I'd rather avoid the functional step altogether and use the structural affines, and given that FreeSurfer is doing nothing but resampling to 256^3 isometric voxels, this shouldn't be fancy. If what we're after is I might still be confused, but I feel like this make more sense. |
|
@effigies - i wasn't saying you should map between functional spaces - since you don't really want to run both registrations. just that the mapping between those two spaces should roughly equal the structural transform as a sanity check. however, each registration has it's own source of error so simply doing a structural transform is not going to be equivalent to running the other registration. the structural transform is still the safest thing to do. |
effigies
force-pushed
the
bbregister
branch
7 times, most recently
from
6617b2a to
8f7cb5d
Compare
|
Wondering if this is causing an issue: Image+centering.pdf The location of the center defined by the affine in the raw images seems to be very different, and the center in the |
|
So this has gone on for quite a while, and I want to try to give a status update, to clear my head and get people's opinions on how best to proceed. I'm testing on ds000005 (give or take some 0's) to avoid LR flip issues in the uh2 datasets. We have been using the following FLIRT sequence (actual nipype filenames simplified): $ flirt -in $EPI -ref $T1 -out init.nii.gz -omat init.mat -dof 6
$ flirt -in $EPI -ref $T1 -omat flirt.mat -searchcost bbr -dof 6 -init init.mat \
-schedule /usr/share/fsl/5.0/etc/flirtsch/bbr.sch -wmseg $WMMASKHere Yielding flirt.mat: We're replacing this with: $ recon-all -s $SUBJ -i $ORIG_T1 -all
$ bbregister --s $SUBJ --mov $EPI --init-coreg --t2 --reg bbreg.dat --fslmat fsl.mat
$ tkregister2 --no-edit --mov $EPI --reg bbreg.dat --targ $T1 --fslregout adjusted.matThis yields fsl.mat (output of bbregister): And adjusted.mat (output of tkregister2): These aren't the same as Aligning these on Freeview shows basically identical images:
So it may be that I've been making too much of differing afffines and transforms, and this is a perfectly acceptable place to be; the aligned functionals are scaled and centered slightly differently, but we have transforms back into the original T1 space. Maybe the best way to test is to determine whether or not the downstream workflow is unable to cope with these differences. What's most likely to get distorted by a messed up Alternately, what's the best way to directly test the transforms?
@chrisfilo @oesteban |
effigies
force-pushed
the
bbregister
branch
2 times, most recently
from
000e193 to
10b05b2
Compare
Use recon_config to feed recon_report
This reverts commit 005da2fef00951277a63e4c1556562e5137e31bd.
- Check final row is almost [0, 0, 0, 1] and set exact - Use decimal notation, not scientific
|
Docs updated. I decided to go with the approach of noting when FreeSurfer adds to or modifies a step in the base workflow, and then describing all surface-based processing in its own section. I suspect that will be smoother when fieldmaps go in, and it allows the FreeSurfer derivatives to be described without cluttering up the main derivatives section. An alternative approach would be to include FreeSurfer stuff by default, and have a section describing the removals/replacements if |
|
there seems to be something wrong with the MNI preproc files: this is the preproc bold in the MNI space overlaid on top of the atlas this is the proproc bold in the T1w space overlaid on top of the T1 This is the normalized T1w overlaid on the MNI ds005 sub-01 I'm gonna check if the same thing happens in master. |
|
The same thing when using master with FLIRT BBR
|
Closes #64