Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Increase FSL serialization precision #144

Merged
merged 3 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions nitransforms/io/fsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class FSLLinearTransform(LinearParameters):
def __str__(self):
"""Generate a string representation."""
lines = [
" ".join(["%g" % col for col in row])
" ".join("%.08f" % col for col in row)
for row in self.structarr["parameters"]
]
return "\n".join(lines + [""])
Expand All @@ -46,14 +46,14 @@ def from_ras(cls, ras, moving=None, reference=None):

# Adjust for reference image offset and orientation
refswp, refspc = _fsl_aff_adapt(reference)
pre = reference.affine.dot(inv(refspc).dot(inv(refswp)))
pre = reference.affine @ inv(refswp @ refspc)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to think of the matrices as aff and swp @ spc. So you are always either aff @ inv(swp @ spc) or swp @ spc @ inv(aff).


# Adjust for moving image offset and orientation
movswp, movspc = _fsl_aff_adapt(moving)
post = inv(movswp).dot(movspc.dot(inv(moving.affine)))
post = movswp @ movspc @ inv(moving.affine)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That catches that here you had movswp inverted and movspc not.


# Compose FSL transform
mat = inv(np.swapaxes(post.dot(ras.dot(pre)), 0, 1))
mat = inv(np.swapaxes(post @ ras @ pre, 0, 1))

tf = cls()
tf.structarr["parameters"] = mat.T
Expand Down Expand Up @@ -92,7 +92,7 @@ def to_ras(self, moving=None, reference=None):
pre = refswp @ refspc @ inv(reference.affine)
# Adjust for moving image offset and orientation
movswp, movspc = _fsl_aff_adapt(moving)
post = moving.affine @ inv(movspc) @ inv(movswp)
post = moving.affine @ inv(movswp @ movspc)
mat = self.structarr["parameters"].T
return post @ np.swapaxes(inv(mat), 0, 1) @ pre

Expand Down
8 changes: 4 additions & 4 deletions nitransforms/tests/data/affine-LPS.fsl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
0.999999 -0.00140494 0.000161717 -3.89014
0.000999999 0.621609 -0.783327 105.905
0.001 0.783327 0.62161 -34.3513
0 0 0 1
0.99999900 -0.00140494 0.00016172 -3.89014000
0.00100000 0.62160900 -0.78332700 105.90500000
0.00100000 0.78332700 0.62161000 -34.35130000
0.00000000 0.00000000 0.00000000 1.00000000
8 changes: 4 additions & 4 deletions nitransforms/tests/data/affine-RAS.fsl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
0.999999 -0.00140494 -0.000161717 4.14529
0.000999999 0.621609 0.783327 -37.3811
-0.001 -0.783327 0.62161 107.976
0 0 0 1
0.99999900 -0.00140494 -0.00016172 4.14529000
0.00100000 0.62160900 0.78332700 -37.38110000
-0.00100000 -0.78332700 0.62161000 107.97600000
0.00000000 0.00000000 0.00000000 1.00000000
8 changes: 4 additions & 4 deletions nitransforms/tests/data/affine-oblique.fsl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
0.999998 -0.00181872 -0.0011965 4.26083
0.00206779 0.621609 0.783325 -25.3129
-0.000680894 -0.783326 0.621611 101.967
0 0 0 1
0.99999800 -0.00181872 -0.00119650 4.26083000
0.00206779 0.62160900 0.78332500 -25.31290000
-0.00068089 -0.78332600 0.62161100 101.96700000
0.00000000 0.00000000 0.00000000 1.00000000
2 changes: 0 additions & 2 deletions nitransforms/tests/test_linear.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,11 @@ def test_loadsave(tmp_path, data_path, testdata_path, fmt):
assert np.allclose(
xfm.matrix,
nitl.load(fname, fmt=fmt, reference=ref_file).matrix,
rtol=1e-2, # FSL incurs into large errors due to rounding
)

assert np.allclose(
xfm.matrix,
nitl.load(fname, fmt=fmt, reference=ref_file, moving=ref_file).matrix,
rtol=1e-2, # FSL incurs into large errors due to rounding
)
else:
assert xfm == nitl.load(fname, fmt=fmt, reference=ref_file)
Expand Down
2 changes: 1 addition & 1 deletion nitransforms/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ def assert_affines_by_filename(affine1, affine2):
else:
xfm1 = np.loadtxt(str(affine1))
xfm2 = np.loadtxt(str(affine2))
np.testing.assert_almost_equal(xfm1, xfm2)
np.allclose(xfm1, xfm2, atol=1e-04)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just lost an assertion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh good catch