[RTM] RF: Miscellaneous registration refactorings #741
Conversation
effigies
force-pushed
the
fsfix_common
branch
3 times, most recently
from
4d74d34 to
ece6657
Compare
effigies
changed the title
There was a problem hiding this comment.
These changes are almost urgent. It is great that without having talked about, we both were thinking towards the same direction :).
I'm considering to create a new workflows.bold folder to split the humongous module we have now. In a different PR, that's for sure. WDYT?
| @@ -542,6 +543,7 @@ def init_surface_recon_wf(omp_nthreads, hires, name='surface_recon_wf'): | |||
| fs_transform = pe.Node( | |||
| fs.Tkregister2(fsl_out='freesurfer2subT1.mat', reg_header=True), | |||
| name='fs_transform') | |||
| fsl2lta = pe.Node(fs.utils.LTAConvert(out_lta=True), name='fsl2lta') | |||
There was a problem hiding this comment.
I would imagine that we don't need FSL transforms because we are not chaining them? (we resample in T1 space, I assume). Is this correct?
There was a problem hiding this comment.
I'm doing this right now because nipype doesn't have LTA outputs from tkregister enabled. See nipy/nipype#2217. (Though it also doesn't matter in the long run, since we drop tkregister in #733.)
There was a problem hiding this comment.
Ugh. Actually just realized I'm missing two inputs to fsl2lta. Hold off on merging.
|
|
||
| # BOLD to T1 transform matrix is from fsl, using c3 tools to convert to | ||
| # something ANTs will like. | ||
| fsl2itk_fwd = pe.Node(c3.C3dAffineTool(fsl2ras=True, itk_transform=True), |
There was a problem hiding this comment.
Great, I was about to propose this change. This is to move c3d into the actual (bbregister or BBR) registration algorithm pipeline, right?
There was a problem hiding this comment.
c3d is being used in the FLIRT-BBR workflow to translate from FSL to ITK.
lta_convert is being used in the bbregister workflow to translate from LTA to ITK. LTA is FreeSurfer's format, and there's no real need to spend time in FSL's space.
|
I've been thinking about splitting |
| @@ -572,12 +574,15 @@ def init_surface_recon_wf(omp_nthreads, hires, name='surface_recon_wf'): | |||
| # reoriented image | |||
| (inputnode, fs_transform, [('t1w', 'target_image')]), | |||
| (autorecon1, fs_transform, [('T1', 'moving_image')]), | |||
| (inputnode, fsl2lta, [('t1w', 'target_file')]), | |||
| (autorecon1, fsl2lta, [('T1', 'source_file')]), | |||
| (fs_transform, fsl2lta, [('fsl_file', 'in_fsl')]), | |||
There was a problem hiding this comment.
@oesteban These are the new lines. lta_convert needs to know the source and target to create a new LTA affine.
|
Sounds good. Feel free to merge when tests pass, or I will if I see it first. (Merge with [skip ci] to avoid the hellishly slow Circle queue, since this branch forks from current master.) |
Because I have three PRs (#694, #733, #740) all working in the same areas of code, to work on one almost invariably causes collisions with the others. This PR simply tracks what I think are uncontroversial changes that have come up while working in one that don't impact the functioning of the others.
fs_2_t1_transformfollow thet1_2_<space>_(forward|reverse)_transformconventionbbregister, uses FreeSurfer's native LTA format until ready to convert to ITKIf this list grows, I will rebase the other PRs against it.