From 5822b4124c499a95b13637a68301331815eb5d58 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Mon, 18 Jul 2022 11:10:33 +0200 Subject: [PATCH 1/5] FIX: Reverse ordering of ITK composite h5 files for ``TransformChain`` This PR makes the ``TransformChain`` class more consistent with the overall coordinate system, assuming that transforms are chained with the *points* criteria. In other words, in the typical setup where we have estimated one initializing affine and then perhaps two levels of nonlinear deformations, when calculating the coordinates of a given index in the reference image, the last nonlinear should be applied first, then the second, and finally the affine to pull information from the moving image. In other words, the chaining (composition) operation works exactly as a single transformation procedure. Resolves #81. --- nitransforms/manip.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nitransforms/manip.py b/nitransforms/manip.py index d4e7e651..91e43b61 100644 --- a/nitransforms/manip.py +++ b/nitransforms/manip.py @@ -131,8 +131,8 @@ def map(self, x, inverse=False): raise TransformError("Cannot apply an empty transforms chain.") transforms = self.transforms - if not inverse: - transforms = self.transforms[::-1] + if inverse: + transforms = list(reversed(self.transforms)) for xfm in transforms: x = xfm(x, inverse=inverse) @@ -157,9 +157,9 @@ def from_filename(cls, filename, fmt="X5", reference=None, moving=None): xforms = itk.ITKCompositeH5.from_filename(filename) for xfmobj in xforms: if isinstance(xfmobj, itk.ITKLinearTransform): - retval.append(Affine(xfmobj.to_ras(), reference=reference)) + retval.insert(0, Affine(xfmobj.to_ras(), reference=reference)) else: - retval.append(DisplacementsFieldTransform(xfmobj)) + retval.insert(0, DisplacementsFieldTransform(xfmobj)) return TransformChain(retval) From 7f1657c9841127a2b3a8eae313670f231942ea16 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Mon, 18 Jul 2022 15:47:07 +0200 Subject: [PATCH 2/5] fix: consistently choose the reference with new convention unit tests are your friend Resolves: #81. --- nitransforms/manip.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nitransforms/manip.py b/nitransforms/manip.py index 91e43b61..44a8ec77 100644 --- a/nitransforms/manip.py +++ b/nitransforms/manip.py @@ -74,8 +74,8 @@ def transforms(self): @transforms.setter def transforms(self, value): self._transforms = _as_chain(value) - if self.transforms[-1].reference: - self.reference = self.transforms[-1].reference + if self.transforms[0].reference: + self.reference = self.transforms[0].reference def append(self, x): """ From ecdda5de8f86b44c5b3a86405390ecfce84bcc2a Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Mon, 18 Jul 2022 16:01:15 +0200 Subject: [PATCH 3/5] fix: also revise asaffine --- nitransforms/manip.py | 18 ++++++++++++++---- nitransforms/tests/test_manip.py | 8 ++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/nitransforms/manip.py b/nitransforms/manip.py index 44a8ec77..416bf198 100644 --- a/nitransforms/manip.py +++ b/nitransforms/manip.py @@ -8,6 +8,7 @@ ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ## """Common interface for transforms.""" from collections.abc import Iterable +import numpy as np from .base import ( TransformBase, @@ -139,10 +140,19 @@ def map(self, x, inverse=False): return x - def asaffine(self): - """Combine a succession of linear transforms into one.""" - retval = self.transforms[-1] - for xfm in self.transforms[:-1][::-1]: + def asaffine(self, indices=None): + """ + Combine a succession of linear transforms into one. + + Parameters + ---------- + indices : :obj:`numpy.array_like` + The indices of the values to extract. + + """ + affines = self.transforms if indices is None else np.take(self.transforms, indices) + retval = affines[0] + for xfm in affines[1:]: retval @= xfm return retval diff --git a/nitransforms/tests/test_manip.py b/nitransforms/tests/test_manip.py index 6dee540e..43ad11d6 100644 --- a/nitransforms/tests/test_manip.py +++ b/nitransforms/tests/test_manip.py @@ -68,16 +68,16 @@ def test_collapse_affines(tmp_path, data_path, ext0, ext1, ext2): """Check whether affines are correctly collapsed.""" chain = TransformChain( [ + Affine.from_filename( + data_path / "regressions" / f"from-scanner_to-bold_mode-image.{ext1}", + fmt=f"{FMT[ext1]}", + ), Affine.from_filename( data_path / "regressions" / f"from-fsnative_to-scanner_mode-image.{ext0}", fmt=f"{FMT[ext0]}", ), - Affine.from_filename( - data_path / "regressions" / f"from-scanner_to-bold_mode-image.{ext1}", - fmt=f"{FMT[ext1]}", - ), ] ) assert np.allclose( From 533a03de5c59cb517babcdd09fdbc044123556f4 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Mon, 18 Jul 2022 17:07:11 +0200 Subject: [PATCH 4/5] enh: add doctest to asaffine() --- nitransforms/linear.py | 19 +++++++++++++++++++ nitransforms/manip.py | 12 ++++++++++++ 2 files changed, 31 insertions(+) diff --git a/nitransforms/linear.py b/nitransforms/linear.py index 00acfafb..84c9126b 100644 --- a/nitransforms/linear.py +++ b/nitransforms/linear.py @@ -13,6 +13,7 @@ from scipy import ndimage as ndi from nibabel.loadsave import load as _nbload +from nibabel.affines import from_matvec from nitransforms.base import ( ImageGrid, @@ -218,6 +219,24 @@ def from_filename(cls, filename, fmt=None, reference=None, moving=None): f"Could not open <{filename}> (formats tried: {', '.join(fmtlist)})." ) + @classmethod + def from_matvec(cls, mat=None, vec=None, reference=None): + """ + Create an affine from a matrix and translation pair. + + Example + ------- + >>> Affine.from_matvec(vec=(4, 0, 0)) # doctest: +NORMALIZE_WHITESPACE + array([[1., 0., 0., 4.], + [0., 1., 0., 0.], + [0., 0., 1., 0.], + [0., 0., 0., 1.]]) + + """ + mat = mat if mat is not None else np.eye(3) + vec = vec if vec is not None else np.zeros((3,)) + return cls(from_matvec(mat, vector=vec), reference=reference) + def __repr__(self): """ Change representation to the internal matrix. diff --git a/nitransforms/manip.py b/nitransforms/manip.py index 416bf198..95090007 100644 --- a/nitransforms/manip.py +++ b/nitransforms/manip.py @@ -144,6 +144,18 @@ def asaffine(self, indices=None): """ Combine a succession of linear transforms into one. + Example + ------ + >>> chain = TransformChain(transforms=[ + ... Affine.from_matvec(vec=(2, -10, 3)), + ... Affine.from_matvec(vec=(-2, 10, -3)), + ... ]) + >>> chain.asaffine() + array([[1., 0., 0., 0.], + [0., 1., 0., 0.], + [0., 0., 1., 0.], + [0., 0., 0., 1.]]) + Parameters ---------- indices : :obj:`numpy.array_like` From e80bd3cbaf37ad48ae0640908e207167a4a1c4c0 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Tue, 19 Jul 2022 11:15:17 +0200 Subject: [PATCH 5/5] fix: indeed, ``asaffine()`` had the reversed convention Thanks Chris for pointing this out. Co-authored-by: Chris Markiewicz --- nitransforms/manip.py | 20 ++++++++++++++++++-- nitransforms/tests/test_manip.py | 8 ++++---- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/nitransforms/manip.py b/nitransforms/manip.py index 95090007..1372ef31 100644 --- a/nitransforms/manip.py +++ b/nitransforms/manip.py @@ -136,7 +136,7 @@ def map(self, x, inverse=False): transforms = list(reversed(self.transforms)) for xfm in transforms: - x = xfm(x, inverse=inverse) + x = xfm.map(x, inverse=inverse) return x @@ -156,6 +156,22 @@ def asaffine(self, indices=None): [0., 0., 1., 0.], [0., 0., 0., 1.]]) + >>> chain = TransformChain(transforms=[ + ... Affine.from_matvec(vec=(1, 2, 3)), + ... Affine.from_matvec(mat=[[0, 1, 0], [0, 0, 1], [1, 0, 0]]), + ... ]) + >>> chain.asaffine() + array([[0., 1., 0., 2.], + [0., 0., 1., 3.], + [1., 0., 0., 1.], + [0., 0., 0., 1.]]) + + >>> np.allclose( + ... chain.map((4, -2, 1)), + ... chain.asaffine().map((4, -2, 1)), + ... ) + True + Parameters ---------- indices : :obj:`numpy.array_like` @@ -165,7 +181,7 @@ def asaffine(self, indices=None): affines = self.transforms if indices is None else np.take(self.transforms, indices) retval = affines[0] for xfm in affines[1:]: - retval @= xfm + retval = xfm @ retval return retval @classmethod diff --git a/nitransforms/tests/test_manip.py b/nitransforms/tests/test_manip.py index 43ad11d6..6dee540e 100644 --- a/nitransforms/tests/test_manip.py +++ b/nitransforms/tests/test_manip.py @@ -68,16 +68,16 @@ def test_collapse_affines(tmp_path, data_path, ext0, ext1, ext2): """Check whether affines are correctly collapsed.""" chain = TransformChain( [ - Affine.from_filename( - data_path / "regressions" / f"from-scanner_to-bold_mode-image.{ext1}", - fmt=f"{FMT[ext1]}", - ), Affine.from_filename( data_path / "regressions" / f"from-fsnative_to-scanner_mode-image.{ext0}", fmt=f"{FMT[ext0]}", ), + Affine.from_filename( + data_path / "regressions" / f"from-scanner_to-bold_mode-image.{ext1}", + fmt=f"{FMT[ext1]}", + ), ] ) assert np.allclose(