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

[WIP; ENH] Storage of iEEG electrodes in subject-specific FreeSurfer space #757

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

adam2392
Copy link
Member

@adam2392 adam2392 commented Mar 15, 2021

Closes: #747

Currently, this is a draft to demonstrate exactly what would fix issues discussed in #747 .

@adam2392
Copy link
Member Author

@sappelhoff not sure what's up w/ the link-checker, i'll take another look today.

Anything holding up this you think?

@sappelhoff
Copy link
Member

sappelhoff commented Mar 23, 2021

So would the workflow for this solution be something like:

README
dataset_description.json
participants.tsv
derivatives/
    freesurfer/
        sub-01/
            T1.mgz <-- based on sub-01_T1w.nii
sub-01/
    anat/
        sub-01_T1w.nii
    ieeg/
        sub-01_space-fsnative_electrodes.tsv
        sub-01_space-fsnative_coordsystem.json <-- IntendedFor contains path to freesurfer T1.mgz in derivatives?

... or am I misunderstanding this?

If I am correct, then we still need to figure out how to appropriately point to a derivatives file, given that they could be stored anywhere and simply using "relative paths from BIDS root" are not guaranteed to work.

cc @effigies

@adam2392
Copy link
Member Author

Yes that is correct!

@adam2392
Copy link
Member Author

Perhaps,

If the derivative data is stored as a subdirectory of `bids_root`, then `IntendedFor` must point to that file relative to `bids_root`. If not, then a DOI should be present that links to that dataset.

?

@adam2392
Copy link
Member Author

adam2392 commented Apr 6, 2021

If the derivative data is stored as a subdirectory of `bids_root`, then `IntendedFor` must point to that file relative to `bids_root`. If not, then a DOI should be present that links to that dataset.

Is there any update with regards to this? Should I include this part, to specify that fsnative and fsnativetkr are derivative coordinate systems?

@adam2392 adam2392 requested a review from sappelhoff April 6, 2021 14:16
@sappelhoff
Copy link
Member

Thanks for the reminder, and sorry that there is so little progress - it seems like very few people are interested in solving this problem.

If the derivative data is stored as a subdirectory of bids_root, then IntendedFor must point to that file relative to bids_root. If not, then a DOI should be present that links to that dataset.

so am I understanding your proposal correctly?

  1. if derivatives/ is part of a BIDS dir, interpret IntendedFor as a relative path from the BIDS dir root
  2. if derivatives/ is NOT part of a BIDS dir, a DOI "should be present" to point to the derivatives/, and then IntendedFor will be used "as if" derivatives/ were nested in the BIDS dir?

then a DOI should be present that links to that dataset.

where should that DOI be present?

What about datasets that do not have a DOI?

@robertoostenveld
Copy link
Collaborator

the proposed fsnativetkr coordinate system is specific to a particular anatomical MRI in FS format. E.g. consider that you start with a 1mm T1 scan and another 0.7 mm T1 scan from the patient. The number of slices and hence the alignment of the first (lower-resolution) scan within the 256x256x256 volume are likely to be different than the number of slices and hence the alignment of the higher-resolution scan. If you run freesurfer on the low-res, the output will be shape-wise very similar to that of the high-res, but the data expressed in the fsnativetkr coordinate system will be shifted with respect to each other.

I think that fsnativetkr therefore cannot be specified without the corresponding IntendedFor linking to an MRI that must represent the 256x256x256 volume in the FS file format.


- `fsnativetkr`: The subject-specific FreeSurfer T1w space. The origin of
the coordinate system is at the center of isotropic 1 mm 256x256x256
volume (that is the voxel center is at slices (128, 128, 128)) in RAS
Copy link
Collaborator

Choose a reason for hiding this comment

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

are these indices 0-offset or 1-offset?

Should the middle of a 256x256x256 volume not be represented as [127.5 127.5 127.5] (when 0-offset) or [128.5 128.5 128.5] (where 1-offset)?

The middle of a 2x2x2 volume would be at voxel [0.5 0.5 0.5] if you start counting voxels at [0 0 0], or at [1.5 1.5 1.5] when you start counting at [1 1 1].

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'm pretty sure it is with 0-offset.

That's what I thought initially... but I think there is some offset, so for example, running the following

        mri_info --vox2ras-tkr <img>

    This will generally be the 4x4 matrix for FreeSurfer output.::

            [
                [-1.0, 0.0, 0.0, 128.0],
                [0.0, 0.0, 1.0, -128.0],
                [0.0, -1.0, 0.0, 128.0],
                [0.0, 0.0, 0.0, 1.0],
            ]

This essentially centers the coordinates at the "center" of the volume "almost".

Reference: https://www.mail-archive.com/freesurfer@nmr.mgh.harvard.edu/msg69541.html

@adam2392
Copy link
Member Author

adam2392 commented Apr 8, 2021

I think that fsnativetkr therefore cannot be specified without the corresponding IntendedFor linking to an MRI that must represent the 256x256x256 volume in the FS file format.

Then perhaps what is needed to fully define this TKR coordinate system is:

"The IntendedFor also MUST be defined linking to an MRI that MUST represent the 256x256x256 volume in the FreeSurfer file format."?

@@ -151,6 +151,20 @@ Restricted keywords for the `<CoordSysType>CoordinateSystem` field in the
[ACPC site](https://www.fieldtriptoolbox.org/faq/acpc/) on the FieldTrip
toolbox wiki.

- `fsnative`: The subject-specific FreeSurfer T1w space. This corresponds to
the original scanner image after it is re-sampled and re-scaled
to an isotropic 1 mm 256x256x256 volume in RAS orientation. The origin
Copy link
Collaborator

Choose a reason for hiding this comment

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

fsnative is not defined relative to the 1mm 256^3 MRI, but is defined relative to the AC of the individual participant.

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'm not sure I follow. Isn't fsnative just the xyz coordinates in RAS space based on the vox2ras affine transformation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

RAS is only the specification of the direction of the axes, the origin also needs to be defined. That origin is at the anterior commissure, or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes you're right. I just noticed I had two incongruent sentences. Adjusted now to specify AC.

@adam2392
Copy link
Member Author

  1. if derivatives/ is part of a BIDS dir, interpret IntendedFor as a relative path from the BIDS dir root
  2. if derivatives/ is NOT part of a BIDS dir, a DOI "should be present" to point to the derivatives/, and then IntendedFor will be used "as if" derivatives/ were nested in the BIDS dir?

then a DOI should be present that links to that dataset.

where should that DOI be present?

What about datasets that do not have a DOI?

@sappelhoff is the best place to handle this within this PR? Perhaps adding a statement:

The `IntendedFor` key must be defined either with: i) a DOI or ii) a relative path from the BIDS directory

?

@sappelhoff
Copy link
Member

is the best place to handle this within this PR? Perhaps adding a statement:

although it will probably delay the efforts here, I think it's best to first deal with that issue here: #471 (comment)

and then come back to the present one.

@sappelhoff
Copy link
Member

@adam2392 -- just a note that #918 has been merged and BIDS URIs will be part of the 1.8 release (probably some time in September). Not sure if what you proposed in this PR is still needed and/or salvageable. If not, please close the PR :-)

Else. we can leave it open and keep working on it.

Copy link
Member

@dorahermes dorahermes left a comment

Choose a reason for hiding this comment

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

Nice, clarifying surface space would be helpful.

volume (that is the voxel center is at slices (128, 128, 128)) in RAS
orientation. This corresponds to FreeSurfer's surface RAS coordinates (TKR coordinates).
The origin MUST be the Anterior Commissure. The ``IntendedFor`` MUST link to an MRI that represents the 256x256x256 volume in the FreeSurfer file format. The difference between this coordinate system and ``fsnative`` is that TKR coordinates are defined on the surface of the brain based on FreeSurfer.

Copy link
Member

Choose a reason for hiding this comment

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

Freesurfer has several surfaces (pial, white, inflated... ), it is unclear which surface is meant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Storing iEEG electrodes with locations from FreeSurfer image
4 participants