[ENH] XForm headers #202

oesteban · 2017-10-07T01:00:04Z

This PR adds utilities to copy xform matrices after registrations or using a new interface. Necessary for nipreps/fmriprep#710

Addressing nipreps/fmriprep#710

…sApplyTransform

oesteban · 2017-10-08T01:43:08Z

Sorry Chris, I've asked your review but we need nipreps/fmriprep#710 to be passing before merging this.

effigies · 2017-10-08T16:33:33Z

Assume you mean #743? And looks like it's passing against the latest commit on this branch.

oesteban · 2017-10-08T21:04:39Z

You're right, I meant nipreps/fmriprep#743. And yes, I've taken a look and it reports look good, registration steps don't seem to be broken.

effigies · 2017-10-08T21:12:28Z

Did it resolve the issue with a dataset exhibiting the problems in nipreps/fmriprep#710?

oesteban · 2017-10-08T21:15:22Z

Sorry, I missed to say the most relevant bit of info. Yes, this #202 and nipreps/fmriprep#743 together do fix nipreps/fmriprep#710.

effigies · 2017-10-08T21:28:17Z

niworkflows/interfaces/utils.py

+def _copyxform(ref_image, out_image, message=None):
+    # Read in reference and output
+    orig = nb.load(ref_image)
+    resampled = nb.load(out_image)


I wonder if it would be a good idea to do some basic check, such as the two images having the same spatial dimensions. I think that's a reasonable precondition to copying an affine matrix, right?

effigies · 2017-10-08T21:43:35Z

niworkflows/interfaces/utils.py

+    sform, sform_code = orig.header.get_sform(coded=True)
+    header = resampled.header.copy()
+    header.set_qform(qform, int(qform_code))
+    header.set_sform(sform, int(sform_code))


The strong assumption being made here is that we're resampling so that, not only are the images in the same RAS space, but in the same IJK (voxel) space, as well. Is this just how ANTs always does things (especially the tools we're "fixing"), or do they ever do an affine update, so that the images are aligned in RAS, but not necessarily in IJK?

This may be the same issue as checking whether they have the same dimensions, so at least by adding that check, we'll reduce the possibility that we are copying an affine to an un-resampled image.

Also, perhaps we should make xform_code an optional parameter. Most of the time with registration, we'll want code 2 (#define NIFTI_XFORM_ALIGNED_ANAT 2 /! Coordinates aligned to another file's, or to anatomical "truth".), while the anatomical image may still have a qform/sform code of 1.

If I understand, the place we should make sure we set the appropriate xform code 2 is at the point we generate the reference for the bold_preproc spaces. All others should inherit the original xforms. Correct?

Actually that code should be 4 when the target is MNI

I would say 90% of the time you're using Registration or ApplyTransform, you're resampling from one image to another, so it should be 2, unless (as you say) it should be 4. There might be cases where you're really not moving, and I think the only example I can think of is the SyN-SDC transform where we go out of our way to bring the reference into the moving image's space to start with.

Also, I don't think you need the CopyXForm interface at all (at least for fmriprep right now). See nipreps/fmriprep#743 (comment).

Opened nipreps/fmriprep#745

Given the precedence (sform > qform > base affine), perhaps the logic we want is:

xform = orig.affine # nibabel will pick the best affine _, qform_code = orig.header.get_qform(coded=True) _, sform_code = orig.header.get_sform(coded=True) xform_code = sform_code if sform_code > 0 else qform_code if xform_code == 1: xform_code = 2 # Keep 0, 2, 3, 4 unchanged header.set_qform(xform, xform_code) header.set_sform(xform, xform_code)

That way, as long as there's a legitimate affine matrix, both the qform and sform will be updated (though qform will drop any shear). Then software that handles sforms badly will have the best hints we can give.

I have added your code as follows: first, I'd move GenerateSamplingReference from fmriprep to niworkflows:

https://github.com/poldracklab/niworkflows/blob/510881813e331c7febe3b1ca43113a999cf3ac70/niworkflows/interfaces/utils.py#L117-L143

and this is how it looks with some modifications:

https://github.com/poldracklab/niworkflows/blob/510881813e331c7febe3b1ca43113a999cf3ac70/niworkflows/interfaces/utils.py#L168-L206

As I said, I think registrations should not change (or fix) the xform matrices. We need to make sure that they are correct when a) conforming inputs and b) generating sampling references. Any other process that happens in a given space should just forward the xforms. That's why I'd be reluctant to replace the copy with a copy&fix xforms.

effigies · 2017-10-08T22:21:32Z

niworkflows/interfaces/utils.py

+    header.set_qform(qform, int(qform_code))
+    header.set_sform(sform, int(sform_code))
+    for quatern in ['b', 'c', 'd']:
+        header['quatern_' + quatern] = orig.header['quatern_' + quatern]


I'm pretty sure this is done in set_qform.

effigies · 2017-10-08T22:25:08Z

niworkflows/interfaces/utils.py

+                                   newpath=runtime.cwd)
+        # Copy and replace header
+        shutil.copy(self.inputs.in_file, out_name)
+        _copyxform(self.inputs.hdr_file, out_name, message='CopyXForm')


Maybe message='CopyXForm; niworkflows v%s' % __version__?

effigies · 2017-10-08T22:26:21Z

niworkflows/interfaces/utils.py

+
+class CopyXForm(SimpleInterface):
+    """
+    Copy a header from the `hdr_file` to `out_file` with data drawn from


We're copying the qform/sform matrices from hdr_file, not the whole header.

effigies · 2017-10-09T01:55:08Z

niworkflows/interfaces/utils.py

+    resampled = nb.load(out_image)
+    orig = nb.load(ref_image)
+
+    if orig.affine != resampled.affine:


I think we should assume the affines might not match; that's why we're copying. I'd say:

if orig.shape[:3] != resampled.shape[:3]:

The problem on nipreps/fmriprep#710 was exactly that the affines matched, but at certain steps, both ANTs and FSL would change the x-forms. But the affines still matched.

This is also to answer the previous comment: we need to make sure that xforms are not changed by accident. The rationale behind this and the accompanying PR in fmriprep is that:

When moving within the same space (SDC, head motion correction, SBRef-EPI), we keep the original xforms (even if they are wrong in the original files).

When moving to different space (anatomical or MNI), we are generating an image taken as reference space. So it is this point when we need to make sure that the xform code is the appropriate one. Since we pass on this reference image to the registration interface, the xform matrices should, indeed, be copied over the resulting images. Same for ApplyTransforms.

Bottomline: no interface should change xforms unless we are certain we want to do that. Only GenerateSamplingReference should make these changes.

If the affines match, what are we correcting? Is it that the sforms matched but the qforms didn't?

effigies · 2017-10-09T02:06:55Z

niworkflows/interfaces/utils.py

+    sform, sform_code = orig.header.get_sform(coded=True)
+    header = resampled.header.copy()
+    header.set_qform(qform, int(qform_code))
+    header.set_sform(sform, int(sform_code))


I would say 90% of the time you're using Registration or ApplyTransform, you're resampling from one image to another, so it should be 2, unless (as you say) it should be 4. There might be cases where you're really not moving, and I think the only example I can think of is the SyN-SDC transform where we go out of our way to bring the reference into the moving image's space to start with.

Also, I don't think you need the CopyXForm interface at all (at least for fmriprep right now). See nipreps/fmriprep#743 (comment).

effigies · 2017-10-09T02:07:45Z

niworkflows/interfaces/fixes.py

+        if out_file is not None and out_file:
+            _copyxform(self.inputs.moving_image[0],
+                       os.path.abspath(out_file),
+                       message=self.__class__.__name__)


Can we add the niworkflows version to the messages in this file, as well?

effigies · 2017-10-09T15:55:51Z

niworkflows/interfaces/utils.py

+    resampled = nb.load(out_image)
+    orig = nb.load(ref_image)
+
+    if np.any(orig.affine != resampled.affine):


If we do want to check affines, I'd prefer this check:

if not np.allclose(orig.affine, resampled.affine):

effigies

Okay. I think I'm happy with this for now. I think perhaps we want to revisit at some point (possibly related to nipreps/fmriprep#475) all of the places we might want to fix an affine, but this is a clear improvement.

effigies · 2017-10-09T16:21:58Z

niworkflows/interfaces/fixes.py

+        if out_file is not None and out_file:
+            _copyxform(self.inputs.fixed_image[0],
+                       os.path.abspath(out_file),
+                       message=self.__class__.__name__)


Sorry, one more place to update message.

effigies · 2017-10-09T16:22:50Z

niworkflows/interfaces/utils.py

+import nilearn.image as nli
+
+from .. import __version__
+from ..nipype import logging
 from niworkflows.nipype.utils.filemanip import fname_presuffix
 from niworkflows.nipype.utils.misc import normalize_mc_params
 from niworkflows.nipype.interfaces.base import (


While we're at it, can we make these ..nipype imports?

effigies · 2017-10-09T16:44:39Z

niworkflows/interfaces/utils.py

+    resampled.header.set_qform(xform, xform_code)
+    resampled.header.set_sform(xform, xform_code)
+    resampled.header['descrip'] = 'reference image generated by %s.' % (
+        message or '(unknown software)')


So all of this... it's just updating the qform_code to match the sform_code, and setting descrip field. Because resampled is being generated by nilearn/nibabel, qform already == sform (modulo shearing):

https://github.com/nipy/nibabel/blob/8c1d0bc4bd9cd757f9b9658547f0e2af55e8ca6c/nibabel/nifti1.py#L1789-L1795

It's not wrong, just I'm not sure what we're buying here.

I see. You are right, this is not buying much. I'd prefer to keep it though, to have your code handy when we need it. Still, the interface now can manually set a different code, which will be useful when we actually start using it.

effigies · 2017-10-09T16:45:34Z

niworkflows/interfaces/utils.py

+    sform, sform_code = orig.header.get_sform(coded=True)
+    header = resampled.header.copy()
+    header.set_qform(qform, int(qform_code))
+    header.set_sform(sform, int(sform_code))


oesteban · 2017-10-09T19:04:18Z

<spooky voice>tests are passing...</spooky voice>

oesteban added 13 commits October 5, 2017 15:38

[ENH] Add a header-safe ants.ApplyTransform interface

7965d8a

Addressing nipreps/fmriprep#710

fix possible deadlock?

b084173

update nipype

2c3b0f2

fix path

65bff06

fix previous commit

0686910

fix ref image

f63c999

fix naming issue of output_name

7addd3d

fix stupid mistake

3e28bf2

update nipype

ee09a51

make Registration interfaces fix headers

642f60d

Merge remote-tracking branch 'upstream/master' into enh/HeaderSafeAnt…

2516595

…sApplyTransform

header-aware interfaces

fee3f74

update nipype

c459164

oesteban mentioned this pull request Oct 7, 2017

[ENH] Ensure that headers are consistent along the process nipreps/fmriprep#743

Merged

oesteban requested a review from effigies October 8, 2017 01:19

fix registration transform, when no output image is required

11921d5

effigies reviewed Oct 8, 2017

View reviewed changes

oesteban added 2 commits October 8, 2017 18:36

update nipype

029daa3

address @effigies' comments

c996291

oesteban mentioned this pull request Oct 9, 2017

Make sure the xform_code is correct when generating references nipreps/fmriprep#745

Closed

effigies reviewed Oct 9, 2017

View reviewed changes

fix affine check, improve message

f39684a

effigies reviewed Oct 9, 2017

View reviewed changes

oesteban added 2 commits October 9, 2017 09:02

add GenerateSamplingReference to niworkflows

315d612

update affine check

5108818

effigies approved these changes Oct 9, 2017

View reviewed changes

oesteban added 2 commits October 9, 2017 10:03

various fixes

9ebfcdf

fix setting xform_code

32b3596

update nipype and use nipy/nipype#2220

1f660df

effigies merged commit c550524 into nipreps:master Oct 10, 2017

oesteban mentioned this pull request Oct 10, 2017

anat affine appears off for ds000114 between data and fmriprep output nipreps/fmriprep#624

Closed

oesteban deleted the enh/HeaderSafeAntsApplyTransform branch October 10, 2017 20:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] XForm headers #202

[ENH] XForm headers #202

oesteban commented Oct 7, 2017

oesteban commented Oct 8, 2017

effigies commented Oct 8, 2017

oesteban commented Oct 8, 2017

effigies commented Oct 8, 2017

oesteban commented Oct 8, 2017

effigies Oct 8, 2017

effigies Oct 8, 2017

oesteban Oct 9, 2017

oesteban Oct 9, 2017

effigies Oct 9, 2017

oesteban Oct 9, 2017

effigies Oct 9, 2017

oesteban Oct 9, 2017

effigies Oct 9, 2017

effigies Oct 8, 2017

effigies Oct 8, 2017

effigies Oct 8, 2017

effigies Oct 9, 2017

oesteban Oct 9, 2017

effigies Oct 9, 2017

effigies Oct 9, 2017

effigies Oct 9, 2017

effigies Oct 9, 2017

effigies left a comment

effigies Oct 9, 2017

effigies Oct 9, 2017

effigies Oct 9, 2017

oesteban Oct 9, 2017 •

edited

Loading

effigies Oct 9, 2017

oesteban commented Oct 9, 2017 •

edited

Loading

[ENH] XForm headers #202

[ENH] XForm headers #202

Conversation

oesteban commented Oct 7, 2017

oesteban commented Oct 8, 2017

effigies commented Oct 8, 2017

oesteban commented Oct 8, 2017

effigies commented Oct 8, 2017

oesteban commented Oct 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

effigies left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oesteban Oct 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oesteban commented Oct 9, 2017 • edited Loading

oesteban Oct 9, 2017 •

edited

Loading

oesteban commented Oct 9, 2017 •

edited

Loading