-
Notifications
You must be signed in to change notification settings - Fork 262
ENH: Adding vox2ras_tkr #164
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
Conversation
tried with 2 MRIs. Results with mri_info match. +1 for merge |
@agramfort do you know what's up with the difference between the freesurfer definition of vox2ras-tkr (which seems to use dC, dR, and dS) and the output of |
Nope sorry |
can this be merged or @Eric89GXL you think it's not ready? @matthew-brett any opinion? |
@agramfort I'm just concerned about the mismatch between what Freesurfer documented, and the results from |
ping @satra any idea? or you can ask doug on the FS mailing list... |
I asked about the transforms at one point, and I was directed to the powerpoint. Part of the issue is that I'm not 100% sure what the dR, dC, dS terms are in the powerpoint. I assumed this was the size of the voxels in each dimension, and assumed this information was stored in |
v2rtkr = np.array([[-1, 0, 0, pcrs_c[0]], | ||
[0, 0, 1, -pcrs_c[2]], | ||
[0, -1, 0, pcrs_c[1]], | ||
[0, 0, 0, 1]], dtype=np.float32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the lines I'm unclear about (not that there are so many to choose from). I expected the 1's in the upper left 3x3 of this matrix to be given by the values in get_zooms()
, but using those values makes the test script fail (since the zooms are not unity, but mri_info --vox2ras-tkr
on the test .mgz file has only +/-1's in the upper left 3x3...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using mri_info
to get the resolution in each direction gives:
$ mri_info --res test.mgz
1.000 1.000 1.000 2.000
However, in nibabel getting the deltas I get:
>>> nib.load('test.mgz').get_header().get_zooms()
(3.7416575, 3.7416575, 3.7416575)
So, clearly, get_zooms() is doing something else that I mis-interpreted. I'm not sure where to get the equivalent information in nibabel, though...
I also shot an email to Doug from the Freesurfer list to ask about how to get the "xsize", "ysize", and "zsize" parameters (which can be obtained from |
can't you obtain the voxels sizes for the get_affine(). You should. |
@agramfort the
And for the test dataset:
I don't see an obvious mapping between the two, and looking at the code and powerpoint, I'm not sure what it would be, either... |
i believe |
@satra yes, but the issue is whether or not it is just a pure rotation (all 1's), or a rotation with scaling (non-1 values). In the Freesurfer coordinate system .ppt, it says these values must be the voxel sizes dR/dC/dS, which happen to be 1 for these datasets, but I don't think have to in general...? Thus I'm trying to figure out where to get the csize/rsize/ssize-equivalent information from the header. |
and then with nibabel:
so we have all the components needed:
to create:
|
Here is the same process for the test.mgz file (which is <100kB if you want to try it):
and then with nibabel:
These would be the corresponding components:
But they don't create:
Thus my confusion... |
I can understand the last column mismatch, since I expected those values just to be |
Actually, both can thus be explained by the mismatch between |
Either that or |
sorry I cannot really help ... but I see you moving forward :) |
I consider this most recent revelation somewhat of a lateral move, actually :) |
at least you're moving :) |
I see from the nibabel code that it's getting the zooms from the the Spacing X, Y, Z fields: http://surfer.nmr.mgh.harvard.edu/fswiki/FsTutorial/MghFormat Is that incorrect? This is what I get for the header info from nibabel:
Dims: (3, 4, 5, 2) MRI Type: MRI_FLOAT goodRASFlag: 1 delta: [ 3.7416575 3.7416575 3.7416575] Mdc: [[ 0.26726124 0.53452247 0.80178374] [ 0.53452247 0.80178374 0.26726124] [ 0.80178374 0.26726124 0.53452247]] Pxyz_c: [ 0. 0. 0.] mrparms: [ 2. 0. 0. 0.] |
@matthew-brett the issue seems to be that those delta values don't match the info obtained using |
@matthew-brett I think I figured it out. The issue is that in |
@matthew-brett, this is what I now get: >>> print(img.get_header())
Dims: (3, 4, 5, 2)
MRI Type: MRI_FLOAT
goodRASFlag: 1
delta: [ 1. 1. 1.]
Mdc:
[[ 1. 2. 3.]
[ 2. 3. 1.]
[ 3. 1. 2.]]
Pxyz_c: [ 0. 0. 0.]
mrparms: [ 2. 0. 0. 0.] Which seems to agree better with
WDYT? To be perfectly honest, I don't understand why the deltas were being re-calculated, but my recent commit makes it so they aren't and it seems to match better... |
@@ -546,7 +558,8 @@ def update_header(self): | |||
if not self._affine is None: | |||
# for more information, go through save_mgh.m in FreeSurfer dist | |||
MdcD = self._affine[:3, :3] | |||
delta = np.sqrt(np.sum(MdcD * MdcD, axis=0)) | |||
delta = hdr['delta'] | |||
#delta = np.sqrt(np.sum(MdcD * MdcD, axis=0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the potentially offending line. Not sure why it's necessary to "re-compute" the deltas if we have them stored, but this re-computation was breaking vox2ras-tkr
equivalence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@satra any idea why the delta values were being re-calculated here? Why is it necessary? They replace the original (correct?) values of hdr['delta']
a couple lines later...
The reason for the update_header command is to deal with the situation where the user updates the affine. The voxel sizes as usually defined are given by np.sqrt(np.sum(affine ** 2, axis=0)) http://mail.scipy.org/pipermail/nipy-devel/2011-March/005696.html I'm trying to understand how this relates to the sizes in the MGH header. |
@matthew-brett assuming the comments I've made in the |
Let's try and stick as close as possible to what mri_info does... I think the best thing is to do what the analyze converters do, and only reset the affine information in 'update_header', if the affine required is significantly different from the affine stored in the header... Sound reasonable? |
That sounds reasonable, but I'm not sure how to define “significantly
|
By the way, the output now matches that of MRIRead.m in the Freesurfer MATLAB toolbox, so maybe we're okay... |
But there is still something funny going on, since the order shouldn't be 1, 2, 0, it should be 0, 2, 1 in the vox2ras-tkr matrix. This is frustrating :( |
We were having trouble with the header updating on load of an image, and changing the underlying header information. nipy#164 Use the Analyze trick to only update the header if the affine has changed.
No we are not resetting the deltas on loading the image, we should get zooms of 1 from the test image.
But I think I've messed up your test :) |
It's annoying that the test.mgz file has a symmetric 3x3 component. I wonder if we're transposing it or something. |
Setting mgh affines
Doug said that the |
By the way, it "breaking" the test actually probably means it's fixed. I had changed it to match the output of MRILoad.m, which wasn't the same as |
Well, tests pass over here with a match to |
I like the comment quote, by the way: |
We weren't previously testing the transpose / not-transpose of the cosine vectors because the test dataset had a symmetical 3x3 in the affine.
:) Added a test for cosine vector order... Are we happy now? Does it all make sense? |
TST: add test for cosine vector order
Yeah, makes sense to me. I'm satisfied with it. |
Tests pass over here, by the way. |
Thanks for taking the time to track all this stuff down... |
Just to check - do you now understand this thing you asked about before? Sorry, I don't... Based on the powerpoint linked from this page (http://surfer.nmr.mgh.harvard.edu/fswiki/CoordinateSystems) I thought that the [-1, -1, 1] triplet that is used to fill in the upper left corner of the matrix would need to be [-delta[0], -delta[2], delta[1]], but that wasn't what the output of mri_info --vox2ras-tkr gave me. |
Yeah, turns out that it was |
OK - makes sense. |
ENH: Adding vox2ras_tkr This adds the support requested in issue #163 for access to the vox2ras_tkr output from the MGH format.
This adds the support requested in #163.
Note that I am not 100% sure about this formulation. Based on the powerpoint linked from this page (http://surfer.nmr.mgh.harvard.edu/fswiki/CoordinateSystems) I thought that the
[-1, -1, 1]
triplet that is used to fill in the upper left corner of the matrix would need to be[-delta[0], -delta[2], delta[1]]
, but that wasn't what the output ofmri_info --vox2ras-tkr
gave me. Can someone confirm before merging?