[FIX] Clarify that BIDS standard template data is to be in scanner coordinates (MEG, iEEG, EEG)

[alexrockhill]

Fixes #1025 (most basic part).

[alexrockhill]

Not sure why the linkcheck failed, looks like it failed accessing the fieldtrip documentation...

[oesteban]

Fixes #1025

After all the conversation on that issue, I'm confused about what the issue is. It seems that there is an issue, but it's not a BIDS-spec issue.

Why only T1s? That is a clear sign that the problem is very specific to your use-case, not a general problem with BIDS.

maybe should consider storing the template T1s somewhere and linking them in the BIDS spec

I don't think BIDS should support this. As @satra mentioned, some BIDS-enthusiasts (including myself) are developing TemplateFlow. If what you need is a programmatic way of retrieving specific metadata associated with a bunch of atlases of interest, that is the IMHO best place to look. Let me know if you need to learn more about templateflow to determine whether it indeed would address your problem.

I'm happy to use the Google Drive link I put up but it might not last forever.

Please, read every template's license first to make sure first you can actually do that, and how you can do that, if you were to do it independently of BIDS. Most of the times you will need to clearly honor attribution clauses, so having obscure links to GDrive doesn't seem a reasonable way of doing that.

[alexrockhill]

After all the conversation on that issue, I'm confused about what the issue is. It seems that there is an issue, but it's not a BIDS-spec issue.

Why only T1s? That is a clear sign that the problem is very specific to your use-case, not a general problem with BIDS.

In my opinion, providing links to websites that only sometimes contain a usable template definition such as a T1 is a problem. A T1 is nice because for software that supports reading BIDS data, this is a standardized way to check that the positions are reasonable.

maybe should consider storing the template T1s somewhere and linking them in the BIDS spec

I don't think BIDS should support this. As @satra mentioned, some BIDS-enthusiasts (including myself) are developing TemplateFlow. If what you need is a programmatic way of retrieving specific metadata associated with a bunch of atlases of interest, that is the IMHO best place to look. Let me know if you need to learn more about templateflow to determine whether it indeed would address your problem.

I think TemplateFlow is a great solution but it's not referenced in the BIDS specification so I think it's reasonable that many BIDS users would not know that it exists. It could be added to BIDS but since it doesn't have all the templates and they are not named the same, that doesn't really solve the problem, although I think that's a great step.

I think @robertoostenveld 's suggestion that we move template data to a derivative directory is the easiest and best fix which would make it less urgent that TemplateFlow have all the BIDS Standard Templates and have all the checks associated with that until more derivatives are checked by BIDS in general. I think it would be easy enough to just move template coordinate frames to derivatives as a follow-up PR to this.

I'm happy to use the Google Drive link I put up but it might not last forever.

Please, read every template's license first to make sure first you can actually do that, and how you can do that, if you were to do it independently of BIDS. Most of the times you will need to clearly honor attribution clauses, so having obscure links to GDrive doesn't seem a reasonable way of doing that.

I haven't checked the licenses and I agree that it isn't the ideal solution but I think having the templates all in one place for internal sharing to discuss them is helpful so that we're all talking about the same thing. I think that's a step for a future PR.

[oesteban]

I think @robertoostenveld 's suggestion that we move template data to a derivative directory

There's currently no specification for templates in BIDS. There's a specification for coordinate systems engendered by templates (yes, typically from T1w images but that's because most of the work has been done on human adults, otherwise it could well be T2w or other schemes) that were created in standard spaces. Please note that BIDS tries to map between coordinates and standard spaces intentionally skipping over templates. This is to say, there is nothing to move, IMHO.

I have heard of previous interest (including myself) in creating a specification for templates. I think it would make sense in derivatives. As the initial ideas didn't get any traction, we pushed towards TemplateFlow.

[alexrockhill]

Hmm okay, I think we're almost on the same page. My issue is that in writing software that reads BIDS-formatted datasets, having channels in a template coordinate frame without the template itself to check that those channel locations make sense and to fully support data with template electrode coordinates is a problem in practice. It sounds like there is a pretty good consensus for moving the templates to derivatives which I can do as a followup PR.

This PR is much simpler, it's just that if coordinates are in terms of a standard template coordinate frame, they should be in RAS and not voxels which I think is consistent with other coordinate frames but not completely obvious because things like AnatomicalLandmarkCoordinates are stored in voxels.

[alexrockhill]

Ok, this is a good first step and happy to debate the best way to implement other, future PRs but LGTM on this one

[oesteban]

My issue is that in writing software that reads BIDS-formatted datasets, having channels in a template coordinate frame without the template itself to check that those channel locations make sense and to fully support data with template electrode coordinates is a problem in practice.

I remain unclear that this is a BIDS issue, and it's even less clear to me after #1025 (where this conversation should be had).

It sounds like there is a pretty good consensus for moving the templates to derivatives which I can do as a followup PR.

Actually, the lack of clear consensus tempered previous initiatives to include templates into the BIDS specs. I can see more support if templates were part of derivatives.

That said, there's no point in saying moving the templates to derivatives because BIDS, as it stands, does not make any prescriptions about templates. It does have COORDINATE systems engendered by templates, and to refer to those coordinate systems it actually names a bunch of templates. So, if someone wants to specify templates as part of BIDS-Derivatives, the effort would start from the ground up.

This PR is much simpler, it's just that if coordinates are in terms of a standard template coordinate frame, they should be in RAS and not voxels which I think is consistent with other coordinate frames but not completely obvious because things like AnatomicalLandmarkCoordinates are stored in voxels.

Where does the spec say that AnatomicalLandmarkCoordinates must be stored in voxels? Because that is something we want to fix. However, I don't think this PR achieves that.

[alexrockhill]

This PR is about specifying that BIDS Standard Template Coordinate Systems are in terms of scanner RAS, I think you're getting a bit off track @oesteban, although I'm happy to discuss future PRs on the referenced issue or on the PRs when they happen.

I assumed someone else that wrote the part of mne-bids that does AnatomicalLandmarkCoordinates had made sure that this was supposed to be in voxels but perhaps that's a mistake. I'll have to look into it.

[alexrockhill]

That said, there's no point in saying moving the templates to derivatives because BIDS, as it stands, does not make any prescriptions about templates. It does have COORDINATE systems engendered by templates, and to refer to those coordinate systems it actually names a bunch of templates. So, if someone wants to specify templates as part of BIDS-Derivatives, the effort would start from the ground up.

To comment on this point though so that it doesn't go unaddressed, the idea would be not to allow iEEGCoordinateSystem, EEGCoordinateSystem or MEGCoordinateSystem to be a BIDS Standard Template Coordinate System outside derivatives, but instead suggest that BIDS users put it in the same place in derivatives (e.g. /users/alex/mne_data/ieeg_bids/sub-1/ieeg/sub-1_space-IXI549Space_coordsystem.json would be suggested to be /users/alex/mne_data/ieeg_bids/derivatives/sub-1/ieeg/sub-1_space-IXI549Space_coordsystem.json).

[alexrockhill]

Here is also where it says AnatomicalLandmarkCoordinates can be in voxels. Sense 1 and sense 2 seems a bit messy here https://bids-specification.readthedocs.io/en/stable/99-appendices/14-glossary.html#anatomicallandmarkcoordinates-sense-2-metadata.

[alexrockhill]

Ok, just checking in on this PR once again, it's not a big change, just a super simple clarification that templates be in scanner RAS. Looking forward to future discussion of moving templates to derivatives etc. but not on this PR, it would be great to get this moving. cc @sappelhoff

[sappelhoff]

@alexrockhill I just asked some @bids-standard/maintainers to please take a look at this. I am a little out of my depths here, especially with Oscar seeming to oppose these changes. Sorry I can't be of more help.

[alexrockhill]

@alexrockhill I just asked some @bids-standard/maintainers to please take a look at this. I am a little out of my depths here, especially with Oscar seeming to oppose these changes. Sorry I can't be of more help.

It wasn't clear to me whether he opposed the change in this PR or proposed future changes, I assume and infer from what he said more the latter although he hasn't approved this PR. I know it takes a long time and requires everyone to get on board for BIDS which requires a lot of patience and I'm not trying to rush things but this is a minuscule change that I don't really understand how it could be opposed since it's just clarifying something that was previously unspecified, which is a bit frustrating that it takes weeks to approve to be honest.

[effigies]

I did not follow the entire discussion in #1025, but reading this PR in light of @oesteban's comment:

Where does the spec say that AnatomicalLandmarkCoordinates must be stored in voxels? Because that is something we want to fix. However, I don't think this PR achieves that.

I think by "scanner coordinates" you mean in the RAS+ coordinates corresponding to the affine stored in some template NIfTI associated with this coordinate system, which presumably should be invariant to which specific template file is selected. I have tried to make that clearer.

I would also say that *CoordinateUnits doesn't apply to all use cases, so I suggest that we make the default interpretation of coordinates "mm". I don't think this would be overly restrictive if this list is ever expanded.

[alexrockhill]

Thanks, changing T1w to NifTI is definitely more inclusive, I didn't realize that was the proposed fix. Also assuming mm seems reasonable.

[effigies]

@oesteban Can you check to see if my suggestion addressed your concerns with this PR?

[oesteban]

This is stated in ll. 193-194 above, I think.

[effigies]

Fair enough. I missed that this was a subheading of Image-based Coordinate Systems.

Suggested change

	If `<CoordSysType>CoordinateUnits` is not defined or applicable, then the units SHALL
	be assumed to be millimeters (mm).

[oesteban]

I don't oppose this PR - it's just I can't favor it and give it a green check if I don't understand the problem (and even less the solution). My understanding of #1025 is that there is a bottleneck implementing some utility in MNE relating to standard-space-generated coordinates systems. Not sure there is an issue with the BIDS spec.

That said, I don't think I understand this section of the BIDS specs at all. The conversations prior to the first drafts were slow, messy, and unclear, and at some point, I gave up. For instance, after reading this part of the spec, I have no idea of what <CoordSysType> in <CoordSysType>CoordinateSystem would be for an image, and whether it would be stored in the coordsystem.json, which seems only defined for (i)EEG. (That without entering into the weirdness of having a key name that can change depending on its semantics - surely a headache for the schema)

For instance, I can't follow the specs regarding having the origin at the ACPC for images and honestly believe the spec just makes unclear something that, by default, was prescribed to be solved by the inherent coordinate system defined by NIfTI (for the case of images).

@alexrockhill introduced ScanRAS, #866, (again, only under iEEG, which I don't get why either) and the reading feels like a repetition of the image-based coordinates found below.

To make it even harder, there's a previous modification (#717, line 94 of the file) making template-engendered coordinates another option for iEEG.

So, if this PR were to modify anything, I would expect it to modify just the ScanRAS definition which seems to be the culprit in the problem sketched in #1025. But, if three other people more acquainted with the specs on coordinate systems see this okay, I'm not going to oppose it, as I said above.

[alexrockhill]

I agree that the iEEG specification is messy and needs some work. I think it should be done piecewise in small increments. The current problem addressed by this PR is how are you supposed to know that your template channel positions shouldn't be shared in voxel coordinates of a template NifTI? It doesn't say anywhere in the spec how they should be stored, just that they should be relative to the template. This clarifies that it should be in RAS+ world/scanner coordinates and not in voxels.

I don't really follow your other comments with regards to ACPC and ScanRAS, perhaps we should open a separate issue and work through those. Again, I would prefer not to discuss them here as I think tackling a large refactoring all at once is preventing any progress from being made.

[oesteban]

I agree that the iEEG specification is messy and needs some work.

I don't know enough to say this. I lean towards the images and templates specs doesn't make much sense.

This clarifies that it should be in RAS+ world/scanner coordinates and not in voxels.

This is not true. It makes it more unclear. If the problem is with templates used with (i)EEG, this clarification should be somewhere under the item added in #717 regarding templates.

It doesn't say anywhere in the spec how they should be stored, just that they should be relative to the template.

Sure, and for some reason, instead of stating it where the specs say that they can be relative to a template, this PR modifies the section about templates itself. Which, as I was saying, is already unclear and unprecise enough.

I don't really follow your other comments with regards to ACPC and ScanRAS, perhaps we should open a separate issue and work through those.

It doesn't really apply to my criticism of this PR, so we can just leave it there. I was just trying to say that I don't fully understand this file of the specification. That said, the ACPC origin and the ScanRAS keywords were given as separate examples and putting them together just makes everything more cloudy.

[alexrockhill]

Ok, @oesteban, is this better?

[oesteban]

Ok, @oesteban, is this better?

Yes, in the sense that it doesn't make the templates section more unclear. Thanks for that.

For (i)EEG, MEG, I don't really know enough to say anything else - so I can't really say yay or nay here.

[alexrockhill]

Okay, sounds good, seems like an improvement by me from the iEEG perspective

[sappelhoff]

it would be great to get @robertoostenveld's opinion on this revised proposal. From my side this looks fine, but we need an additional (i)MEEG person who has regularly worked with such data to agree as well.

[sappelhoff]

@dorahermes @robertoostenveld are you fine with this change?

[robertoostenveld]

I agree with the suggested change.