mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.67k stars 1.31k forks source link

Scaling identified in the coregistration does not become part of the transformation matrix #10410

Open VoodooCode14 opened 2 years ago

VoodooCode14 commented 2 years ago

Bug description

Setting the flag coreg.set_scale_mode("3-axis") allows for scaling when computing the coregistration parameters. For some reason, those parameters do later on not become part of the transformation matrix. Instead they appear to be applied to the MRI points for reasons I cannot follow (which are not explained in the code either). Hence, when a head is matched to a different MRI (like fsaverage) and the head is thicker or smaller than the MRI head, the registration points cannot be on the head surface.

This can easily be fixed with the following code: coreg.trans["trans"][0, 0] = coreg.trans["trans"][0, 0] 1/coreg.scale[0] coreg.trans["trans"][1, 1] = coreg.trans["trans"][1, 1] 1/coreg.scale[1] coreg.trans["trans"][2, 2] = coreg.trans["trans"][2, 2] * 1/coreg.scale[2]

Yet, this is not a proper fix, but merely a quick hack to solve the issue.

When plotting the visualization before applying the fix using the following command _mne.viz.plotalignment() most of the registration points were within the head instead of being on top.

Furthermore, when later on estimating the forward model, only the transformation matrix from the coregistration object is used, hence it appears that any scaling information is erroneously lost.

Bug impact

Since the visualization is incorrect (without the aforementioned correction) and the forward model is merely provided with the transformation matrix, I am now suspecting that every transformation into source space based on fsaverage is affected. If this is the case, please confirm.

Steps to reproduce

1) Run the coregistration on a head bigger (or smaller) than fsaverage and use fsaverage's MRI information. 2) Fit the registration points to the MRI using fit_fiducials() and fit_icp() 3) Use mne.viz.plot_alignment() to plot the points within the head.

Expected results

I would have expected for the scaling information to be included in the transformation matrix, but this does not happen.

Actual results

The scaling information is not included in the transformation matrix from the coregistration, resulting in errors during visualization and further down the line.

Additional information

The documentation on the coregistration on the website is incorrect and misleading. Exemplary, fit_icp says that it "Find MRI scaling, translation, and rotation to match HSP.", but by default "set_scale_mode" is set to None which means no scaling.

welcome[bot] commented 2 years ago

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

larsoner commented 2 years ago

Setting the flag coreg.set_scale_mode("3-axis") allows for scaling when computing the coregistration parameters.

This is not quite the idea. The idea is that the scale mode (uniform or 3-axis) scales the entire FreeSurfer subject and saves this to disk as a new FreeSurfer subject when you press the "Save scaled anatomy" button (e.g., fsaverage_warped_subject13). This new subject should then be used in conjunction with the head<->MRI coregistration matrix for forward/inverse/source modeling. Then at the level of SourceEstimate, you can think of your data as being in the source space of your original surrogate subject (e.g., fsaverage) or in the scaled MRI space (fsaverage_warped_subject13), whichever is more convenient.

The head<->MRI transform should only ever be a rigid (rot + trans) transformation, and the one that you get when using a scaled MRI should properly align the head space to the saved-to-disk scaled MRI subject MRI.

VoodooCode14 commented 2 years ago

Thank you very much for the quick reply.

I have compared the output of the way you mentioned, saving the new anatomy and subsequently using this one for modeling. In the end, the results (mne.viz.plot_alignment) appeared to be equal to me when saving and loading the warped anatomy compared to incorporating the zoom results into the transition matrix.

On another note, I was wondering whether there is a disadvantage from incorporating the zoom into the transition matrix? Currently, it looks for me as if saving the warped anatomy and loading it later on, instead of incorporating the zoom into the transition matrix, requires secret (for lack of a better term) knowledge in how to handle that.

I guess the label of this issue should be changed from bug to enhancement.

larsoner commented 2 years ago

On another note, I was wondering whether there is a disadvantage from incorporating the zoom into the transition matrix?

Yes, there are likely multiple places in the code that we assume the trans is a rigid-body transformation. At least one important one is the forward modeling code. For example, the surface normals are rotated (but not translated) under the assumption that we can get just the rotation by multiplying by the upper left 3x3 (and still end up with a unit vector), which will not be the case if we incorporate the scaling into the transformation matrix. All of our surface transformation code works like this, so plot_alignment will probably also fail similarly, given the right arguments / things to plot (like source spaces).

Rather than try to add another way of doing the same thing here (via scale factors in the trans), I'd rather document the existing workflow better