From 755e8d1c18b948a6efee262419feae77054ea3ad Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 16 Nov 2023 17:11:10 +0100 Subject: [PATCH 1/7] fix: inefficient iterative reloading of reference and moving images The list comprehension had ``nb.loads`` and other method calls continously happening when converting the affine array into RAS. --- nitransforms/io/afni.py | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/nitransforms/io/afni.py b/nitransforms/io/afni.py index b7fc657b..75476e77 100644 --- a/nitransforms/io/afni.py +++ b/nitransforms/io/afni.py @@ -108,17 +108,24 @@ def from_string(cls, string): sa["parameters"] = parameters return tf - def to_ras(self, moving=None, reference=None): + def to_ras(self, moving=None, reference=None, pre_rotation=None, post_rotation=None): """Return a nitransforms internal RAS+ matrix.""" # swapaxes is necessary, as axis 0 encodes series of transforms retval = LPS @ np.swapaxes(self.structarr["parameters"].T, 0, 1) @ LPS - reference = _ensure_image(reference) - if reference is not None and _is_oblique(reference.affine): - retval = retval @ _cardinal_rotation(reference.affine, True) - moving = _ensure_image(moving) - if moving is not None and _is_oblique(moving.affine): - retval = _cardinal_rotation(moving.affine, False) @ retval + if pre_rotation is None and reference is not None: + ref_aff = _ensure_image(reference).affine + pre_rotation = _cardinal_rotation(ref_aff, True) if _is_oblique(ref_aff) else None + + if pre_rotation is not None: + retval = retval @ pre_rotation + + if post_rotation is None and reference is not None: + mov_aff = _ensure_image(moving).affine + post_rotation = _cardinal_rotation(mov_aff, True) if _is_oblique(mov_aff) else None + + if post_rotation is not None: + retval = post_rotation @ retval return retval @@ -130,9 +137,21 @@ class AFNILinearTransformArray(BaseLinearTransformList): def to_ras(self, moving=None, reference=None): """Return a nitransforms' internal RAS matrix.""" - return np.stack( - [xfm.to_ras(moving=moving, reference=reference) for xfm in self.xforms] - ) + + pre_rotation = None + if reference is not None: + ref_aff = _ensure_image(reference).affine + pre_rotation = _cardinal_rotation(ref_aff, True) if _is_oblique(ref_aff) else None + + post_rotation = None + if moving is not None: + mov_aff = _ensure_image(moving).affine + post_rotation = _cardinal_rotation(mov_aff, True) if _is_oblique(mov_aff) else None + + return np.stack([ + xfm.to_ras(pre_rotation=pre_rotation, post_rotation=post_rotation) + for xfm in self.xforms + ]) def to_string(self): """Convert to a string directly writeable to file.""" From 6b0f3a8883f773cd01cbe7460759f952d6dac93f Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 16 Nov 2023 17:29:49 +0100 Subject: [PATCH 2/7] Update nitransforms/io/afni.py Co-authored-by: Chris Markiewicz --- nitransforms/io/afni.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/nitransforms/io/afni.py b/nitransforms/io/afni.py index 75476e77..5afa1e25 100644 --- a/nitransforms/io/afni.py +++ b/nitransforms/io/afni.py @@ -138,18 +138,15 @@ class AFNILinearTransformArray(BaseLinearTransformList): def to_ras(self, moving=None, reference=None): """Return a nitransforms' internal RAS matrix.""" - pre_rotation = None - if reference is not None: - ref_aff = _ensure_image(reference).affine - pre_rotation = _cardinal_rotation(ref_aff, True) if _is_oblique(ref_aff) else None - - post_rotation = None - if moving is not None: - mov_aff = _ensure_image(moving).affine - post_rotation = _cardinal_rotation(mov_aff, True) if _is_oblique(mov_aff) else None + pre_rotation = post_rotation = np.eye(4) + + if reference is not None and _is_oblique(ref_aff := _ensure_image(reference).affine): + pre_rotation = _cardinal_rotation(ref_aff, True) + if moving is not None and _is_oblique(mov_aff := _ensure_image(moving).affine): + post_rotation = _cardinal_rotation(mov_aff, True) return np.stack([ - xfm.to_ras(pre_rotation=pre_rotation, post_rotation=post_rotation) + post_rotation @ xfm.to_ras() @ pre_rotation for xfm in self.xforms ]) From 4ace2bebb6359f31c79fa27650d76c8e82171354 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 16 Nov 2023 17:52:55 +0100 Subject: [PATCH 3/7] enh: remove unnecessary new arguments Co-authored-by: Chris Markiewicz --- nitransforms/io/afni.py | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/nitransforms/io/afni.py b/nitransforms/io/afni.py index 5afa1e25..ebe19703 100644 --- a/nitransforms/io/afni.py +++ b/nitransforms/io/afni.py @@ -108,26 +108,23 @@ def from_string(cls, string): sa["parameters"] = parameters return tf - def to_ras(self, moving=None, reference=None, pre_rotation=None, post_rotation=None): + def to_ras(self, moving=None, reference=None): """Return a nitransforms internal RAS+ matrix.""" # swapaxes is necessary, as axis 0 encodes series of transforms - retval = LPS @ np.swapaxes(self.structarr["parameters"].T, 0, 1) @ LPS - - if pre_rotation is None and reference is not None: - ref_aff = _ensure_image(reference).affine - pre_rotation = _cardinal_rotation(ref_aff, True) if _is_oblique(ref_aff) else None - - if pre_rotation is not None: - retval = retval @ pre_rotation - if post_rotation is None and reference is not None: - mov_aff = _ensure_image(moving).affine - post_rotation = _cardinal_rotation(mov_aff, True) if _is_oblique(mov_aff) else None - - if post_rotation is not None: - retval = post_rotation @ retval + pre_rotation = post_rotation = np.eye(4) + if reference is not None and _is_oblique(ref_aff := _ensure_image(reference).affine): + pre_rotation = _cardinal_rotation(ref_aff, True) + if moving is not None and _is_oblique(mov_aff := _ensure_image(moving).affine): + post_rotation = _cardinal_rotation(mov_aff, True) - return retval + return ( + post_rotation + @ LPS + @ np.swapaxes(self.structarr["parameters"].T, 0, 1) + @ LPS + @ pre_rotation + ) class AFNILinearTransformArray(BaseLinearTransformList): @@ -139,7 +136,6 @@ def to_ras(self, moving=None, reference=None): """Return a nitransforms' internal RAS matrix.""" pre_rotation = post_rotation = np.eye(4) - if reference is not None and _is_oblique(ref_aff := _ensure_image(reference).affine): pre_rotation = _cardinal_rotation(ref_aff, True) if moving is not None and _is_oblique(mov_aff := _ensure_image(moving).affine): From 3a5467f19ba0e2e3054317c07731127ccbb731b8 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 16 Nov 2023 18:07:29 +0100 Subject: [PATCH 4/7] fix: roll back original implementation --- nitransforms/io/afni.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/nitransforms/io/afni.py b/nitransforms/io/afni.py index ebe19703..1e878563 100644 --- a/nitransforms/io/afni.py +++ b/nitransforms/io/afni.py @@ -111,20 +111,16 @@ def from_string(cls, string): def to_ras(self, moving=None, reference=None): """Return a nitransforms internal RAS+ matrix.""" # swapaxes is necessary, as axis 0 encodes series of transforms + retval = LPS @ np.swapaxes(self.structarr["parameters"].T, 0, 1) @ LPS + reference = _ensure_image(reference) + if reference is not None and _is_oblique(reference.affine): + retval = retval @ _cardinal_rotation(reference.affine, True) - pre_rotation = post_rotation = np.eye(4) - if reference is not None and _is_oblique(ref_aff := _ensure_image(reference).affine): - pre_rotation = _cardinal_rotation(ref_aff, True) - if moving is not None and _is_oblique(mov_aff := _ensure_image(moving).affine): - post_rotation = _cardinal_rotation(mov_aff, True) + moving = _ensure_image(moving) + if moving is not None and _is_oblique(moving.affine): + retval = _cardinal_rotation(moving.affine, False) @ retval - return ( - post_rotation - @ LPS - @ np.swapaxes(self.structarr["parameters"].T, 0, 1) - @ LPS - @ pre_rotation - ) + return retval class AFNILinearTransformArray(BaseLinearTransformList): From 4c8f0437cda4d9f7224006271fc70d8ddeadcdfc Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 16 Nov 2023 20:13:05 +0100 Subject: [PATCH 5/7] enh: add test to cover new lines --- nitransforms/tests/data/affine-RAS.afni-array | 3 +++ nitransforms/tests/test_io.py | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 nitransforms/tests/data/affine-RAS.afni-array diff --git a/nitransforms/tests/data/affine-RAS.afni-array b/nitransforms/tests/data/affine-RAS.afni-array new file mode 100644 index 00000000..df023e21 --- /dev/null +++ b/nitransforms/tests/data/affine-RAS.afni-array @@ -0,0 +1,3 @@ +# 3dvolreg matrices (DICOM-to-DICOM, row-by-row): +0.999999 -0.000999999 -0.001 -4 0.00140494 0.621609 0.783327 -2 -0.000161717 -0.783327 0.62161 -1 +0.999999 -0.000999999 -0.001 -4 0.00140494 0.621609 0.783327 -2 -0.000161717 -0.783327 0.62161 -1 diff --git a/nitransforms/tests/test_io.py b/nitransforms/tests/test_io.py index 2475c946..cb9270d7 100644 --- a/nitransforms/tests/test_io.py +++ b/nitransforms/tests/test_io.py @@ -180,7 +180,7 @@ def test_LT_conversions(data_path, fname): "oblique", ], ) -@pytest.mark.parametrize("sw", ["afni", "fsl", "fs", "itk"]) +@pytest.mark.parametrize("sw", ["afni", "fsl", "fs", "itk", "afni-array"]) def test_Linear_common(tmpdir, data_path, sw, image_orientation, get_testdata): tmpdir.chdir() @@ -206,6 +206,9 @@ def test_Linear_common(tmpdir, data_path, sw, image_orientation, get_testdata): fname = f"affine-{image_orientation}.{sw}{ext}" + if sw == "afni-array": + fname.replace(image_orientation, "RAS") + # Test the transform loaders are implemented xfm = factory.from_filename(data_path / fname) @@ -222,6 +225,9 @@ def test_Linear_common(tmpdir, data_path, sw, image_orientation, get_testdata): # Test from_ras RAS = from_matvec(euler2mat(x=0.9, y=0.001, z=0.001), [4.0, 2.0, -1.0]) + if sw == "afni-array": + RAS = [RAS, RAS] + xfm = factory.from_ras(RAS, reference=reference, moving=moving) assert np.allclose(xfm.to_ras(reference=reference, moving=moving), RAS) From 0a02bcf8fb7cc73f765655cebf4d821134ce370a Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 16 Nov 2023 20:17:25 +0100 Subject: [PATCH 6/7] fix: error in new test case implementation --- nitransforms/tests/test_io.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nitransforms/tests/test_io.py b/nitransforms/tests/test_io.py index cb9270d7..982167f5 100644 --- a/nitransforms/tests/test_io.py +++ b/nitransforms/tests/test_io.py @@ -190,6 +190,8 @@ def test_Linear_common(tmpdir, data_path, sw, image_orientation, get_testdata): ext = "" if sw == "afni": factory = afni.AFNILinearTransform + elif sw == "afni-array": + factory = afni.AFNILinearTransformArray elif sw == "fsl": factory = fsl.FSLLinearTransform elif sw == "itk": From 01cfcbc0b976f8e18e492f5a07d5ec20a6af6da2 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 16 Nov 2023 21:17:43 +0100 Subject: [PATCH 7/7] fix: bug in oblique adjusting matrices computation --- nitransforms/io/afni.py | 16 ++++++++++++---- nitransforms/tests/data/affine-LAS.afni-array | 1 + nitransforms/tests/data/affine-LPS.afni-array | 1 + .../tests/data/affine-oblique.afni-array | 1 + nitransforms/tests/test_io.py | 5 +---- 5 files changed, 16 insertions(+), 8 deletions(-) create mode 120000 nitransforms/tests/data/affine-LAS.afni-array create mode 120000 nitransforms/tests/data/affine-LPS.afni-array create mode 120000 nitransforms/tests/data/affine-oblique.afni-array diff --git a/nitransforms/io/afni.py b/nitransforms/io/afni.py index 1e878563..b197c4ed 100644 --- a/nitransforms/io/afni.py +++ b/nitransforms/io/afni.py @@ -135,10 +135,10 @@ def to_ras(self, moving=None, reference=None): if reference is not None and _is_oblique(ref_aff := _ensure_image(reference).affine): pre_rotation = _cardinal_rotation(ref_aff, True) if moving is not None and _is_oblique(mov_aff := _ensure_image(moving).affine): - post_rotation = _cardinal_rotation(mov_aff, True) + post_rotation = _cardinal_rotation(mov_aff, False) return np.stack([ - post_rotation @ xfm.to_ras() @ pre_rotation + post_rotation @ (xfm.to_ras() @ pre_rotation) for xfm in self.xforms ]) @@ -152,14 +152,22 @@ def to_string(self): if line.strip() ] strings += lines - return "\n".join(strings) + return "\n".join(strings + [""]) @classmethod def from_ras(cls, ras, moving=None, reference=None): """Create an ITK affine from a nitransform's RAS+ matrix.""" _self = cls() + + pre_rotation = post_rotation = np.eye(4) + + if reference is not None and _is_oblique(ref_aff := _ensure_image(reference).affine): + pre_rotation = _cardinal_rotation(ref_aff, False) + if moving is not None and _is_oblique(mov_aff := _ensure_image(moving).affine): + post_rotation = _cardinal_rotation(mov_aff, True) + _self.xforms = [ - cls._inner_type.from_ras(ras[i, ...], moving=moving, reference=reference) + cls._inner_type.from_ras(post_rotation @ ras[i, ...] @ pre_rotation) for i in range(ras.shape[0]) ] return _self diff --git a/nitransforms/tests/data/affine-LAS.afni-array b/nitransforms/tests/data/affine-LAS.afni-array new file mode 120000 index 00000000..27d48851 --- /dev/null +++ b/nitransforms/tests/data/affine-LAS.afni-array @@ -0,0 +1 @@ +affine-RAS.afni-array \ No newline at end of file diff --git a/nitransforms/tests/data/affine-LPS.afni-array b/nitransforms/tests/data/affine-LPS.afni-array new file mode 120000 index 00000000..27d48851 --- /dev/null +++ b/nitransforms/tests/data/affine-LPS.afni-array @@ -0,0 +1 @@ +affine-RAS.afni-array \ No newline at end of file diff --git a/nitransforms/tests/data/affine-oblique.afni-array b/nitransforms/tests/data/affine-oblique.afni-array new file mode 120000 index 00000000..27d48851 --- /dev/null +++ b/nitransforms/tests/data/affine-oblique.afni-array @@ -0,0 +1 @@ +affine-RAS.afni-array \ No newline at end of file diff --git a/nitransforms/tests/test_io.py b/nitransforms/tests/test_io.py index 982167f5..bcee9198 100644 --- a/nitransforms/tests/test_io.py +++ b/nitransforms/tests/test_io.py @@ -208,9 +208,6 @@ def test_Linear_common(tmpdir, data_path, sw, image_orientation, get_testdata): fname = f"affine-{image_orientation}.{sw}{ext}" - if sw == "afni-array": - fname.replace(image_orientation, "RAS") - # Test the transform loaders are implemented xfm = factory.from_filename(data_path / fname) @@ -228,7 +225,7 @@ def test_Linear_common(tmpdir, data_path, sw, image_orientation, get_testdata): # Test from_ras RAS = from_matvec(euler2mat(x=0.9, y=0.001, z=0.001), [4.0, 2.0, -1.0]) if sw == "afni-array": - RAS = [RAS, RAS] + RAS = np.array([RAS, RAS]) xfm = factory.from_ras(RAS, reference=reference, moving=moving) assert np.allclose(xfm.to_ras(reference=reference, moving=moving), RAS)