-
Notifications
You must be signed in to change notification settings - Fork 165
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
[ENH] Allow encoding the fieldmapping intent of the protocol #622
[ENH] Allow encoding the fieldmapping intent of the protocol #622
Conversation
@@ -70,8 +70,11 @@ reflected in a single DICOM tag for all possible aforementioned scan | |||
manipulations). See [here](https://lcni.uoregon.edu/kb-articles/kb-0003) and | |||
[here](https://github.com/neurolabusc/dcm_qa/tree/master/In/TotalReadoutTime) | |||
|
|||
<sup>3</sup>We use the "FSL definition", i.e, the time between the center of the |
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 mentioned again under fieldmaps. I've moved that mention (after applying some improvements to the language) over here for completeness.
@@ -104,6 +107,24 @@ Useful for multimodal co-registration with MEG, (S)EEG, TMS, etc. | |||
| ----------------------------- | --------------------- | ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | |||
| AnatomicalLandmarkCoordinates | RECOMMENDED | [object][] of [arrays][] | Key:value pairs of any number of additional anatomical landmarks and their coordinates in voxel units (where first voxel has index 0,0,0) relative to the associated anatomical MRI, (e.g. `{"AC": [127,119,149], "PC": [128,93,141], "IH": [131,114,206]}, or {"NAS": [127,213,139], "LPA": [52,113,96], "RPA": [202,113,91]}`). | | |||
|
|||
### Echo-Planar Imaging & B0 field-mapping |
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 is the core of the proposal
src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md
Outdated
Show resolved
Hide resolved
Addresses the use case of DWI sequencies that need to be split in several runs because, e.g., the scanner would exceed temperature specifications due to fast gradient-switching. This PR also refactors the text of DWIs to better accomodate the addition and make DWIs more consistent with the rest of the file. The approach to solve this is similar to that of bids-standard#622 for fieldmaps. References: nipreps/dmriprep#43.
src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md
Outdated
Show resolved
Hide resolved
src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md
Outdated
Show resolved
Hide resolved
Addresses the use case of DWI sequencies that need to be split in several runs because, e.g., the scanner would exceed temperature specifications due to fast gradient-switching. This PR also refactors the text of DWIs to better accomodate the addition and make DWIs more consistent with the rest of the file. The approach to solve this is similar to that of bids-standard#622 for fieldmaps. References: nipreps/dmriprep#43.
Addresses the use case of DWI sequencies that need to be split in several runs because, e.g., the scanner would exceed temperature specifications due to fast gradient-switching. This PR also refactors the text of DWIs to better accomodate the addition and make DWIs more consistent with the rest of the file. The approach to solve this is similar to that of bids-standard#622 for fieldmaps. References: nipreps/dmriprep#43.
0fcc838
to
953ed10
Compare
Good catch. It looks like |
Are any code owners willing to leave a green tick to get this merged? We will need two more (and/or give Matt rights, how does that process work?) @bids-standard/raw-mri-dwi @bids-standard/raw-mri |
✅ |
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.
Everything looks good to me.
This PR has the necessary approvals at this point, I think, but since we're in the embargo period before a release, we can't merge this until Friday, at least. |
@oesteban Can you check that merge? I think I may have done it backwards. |
The merge looks good, with the latest additions to Should I just merge? |
@oesteban I think I restored something you intended to remove. I'm going to re-do the merge. |
b0312f8
to
234b5b8
Compare
234b5b8
to
8d862e7
Compare
Here's the change that I accidentally dropped: https://github.com/bids-standard/bids-specification/compare/b0312f8a6d1daa2d3683bc45bbd9e9f65d232c43..8d862e79d42390094e8cfa3ca75cedba2d4df5a2 |
* Draft schema for suffixes. * Delete Untitled.ipynb * Clean up suffixes from Anatomical MRI's first table. * Add second table from Anatomical MRI. * Add third table from Anatomical MRI. * Add first table from Task imaging data * Add DWI and B0 field map suffixes. * Add anatomical qMRI table suffixes * Add field map qMRI table suffixes * Add ASL suffixes. * Add MEG suffixes. * Add EEG suffixes. * Add remaining suffixes. * Fix T1map. * Add make_suffix_table. * Add angio to first suffix table. * Add suffix info to README. * Add descriptions for remaining suffixes. * Update fieldmap suffix definition from #622. * Fix bad merge. * Use three underscores, not two. * Apply suggestions from code review Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Further address review. * Apply suggestions from code review Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Remove hardcoded tables. Co-authored-by: Chris Markiewicz <effigies@gmail.com>
This PR addresses the problem @mattcieslak spotted at in #239.
This enhancement (WIP) basically allows for researchers to encode the protocol's intent regarding fieldmaps.
As @satra introduced in #239 (comment), BIDS "could encode intent and automation. Whether it should is a community decision."
This PR proposes a solution to encoding the intent. It doesn't modify anything to allow also encoding automation.
The PR attempts to be backward compatible.
I'm submitting this draft PR to open discussions and looking forward to feedback.
Resolves: #239.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
The gist of this PR...
... is the creation of two new metadata pairs
B0FieldIdentifier
andB0FieldSource
to encode how the researcher originally designed the MR protocol regarding fieldmaps.B0FieldIdentifier
is set in all images that are to be used in estimating some fieldmap. Images with the sameB0FieldIdentifier
are expected to be used together (e.g., two_epi
datasets underfmap/
with opposed PE direction, two_epi
datasets underfmap/
plus two_dwi
datasets underdwi/
, or maybe two_sbref
datasets underfunc/
with opposed PEs, etc.)B0FieldSource
states identifiers intended for correcting the particular instance where it is defined.Currently unsupported example:
would have the following metadata:
sub-01_task-rest_bold.json:
sub-01_task-rest_dir-ap_sbref.json
and sub-01_task-rest_dir-pa_sbref.json
A currently supported example:
would have the following metadata:
sub-01_task-rest_bold.json:
sub-01_phasediff.json