Skip to content

Conversation

@alexrockhill
Copy link
Contributor

@alexrockhill alexrockhill commented Mar 2, 2022

Fixes #961
Fixes #962

So the issue was that groups wanted to use the standard template coordinate frames but they are currently just mapped to other and then we ask them to change it manually. We have these frames already in the config file so the solution with this PR is to set them to other in the raw object returned by read_raw_bids and then explain to them that MNE does not recognize these coordinate frames so they will be treated as unknown but you can basically treat it like mri and find the fiducials and make a head->template trans.

The channels and coordinate system writing and reading is a bit of a jalopy at the moment with a bunch of things added and it's really hard to follow so I simplified but this will undoubtably break all the tests which I haven't fixed yet.

I'm pushing this now to ask for advice on what's going on with the ieeg example. In the last section, I set a transform, it gets saved in those transformed coordinates, I add back the fiducials and compute_native_head_t but it's way off:

Original: <Transform | head->unknown>
[[  1.    0.1   0.2  10.1]
 [ -0.1   1.    0.1 -20.3]
 [  0.2  -0.1   1.  -30.7]
 [  0.    0.    0.    1. ]]
Recovered: <Transform | unknown->head>
[[ 0.97590007 -0.09759001  0.19518001 -5.84352394]
 [ 0.11789673  0.98844615 -0.09526055 15.95020087]
 [-0.18362846  0.11597587  0.97613021 34.17615497]
 [ 0.          0.          0.          1.        ]]

No idea what's going on there, maybe someone who wrote montage code could help a bit.

Merge checklist

Maintainer, please confirm the following before merging:

  • All comments resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • PR description includes phrase "closes <#issue-number>"

@alexrockhill
Copy link
Contributor Author

Okay, I'm not sure if this PR is making things better, it's taking on too much complication all at once, I might have to go back and implement this refactoring piecewise. I've definitely noticed a lot of bugs/typos and especially cases that aren't covered that I do cover in this draft. It passes the test I wrote so that's a good step, it should fail everything else though. I'll try and revisit tomorrow and see if I can't get things to work.

@alexrockhill alexrockhill mentioned this pull request Mar 3, 2022
6 tasks
@hoechenberger hoechenberger added this to the 0.11 milestone Mar 7, 2022
@alexrockhill alexrockhill mentioned this pull request Mar 7, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Montage coordinate frames are saved wrongly

2 participants