bids-specification
bids-specification copied to clipboard
[ENH] Add noRF suffix for MR excitation-free noise scans
Closes #1024.
Changes proposed:
- Add noRF suffix to schema.
@handwerkerd I'd love your input on the description.
This LGTM. It seems like this needs to be done manually, but worth looping in @neurolabusc and @yarikoptic for any possible implications for dcm2niix/heudiconv.
@tsalo can you provide exemplar DICOM validation datasets as sourcedata? It would be great to see examples from the major manufacturers (GE, Siemens, Philips). It is very hard to support a specification without concrete examples. BIDS fields are often defined by experts that have a clear vision of what is needed, but there can be unintended consequences if the implementation is left to end users that have no idea what this data looks like.
@tsalo can you provide exemplar DICOM validation datasets as sourcedata? It would be great to see examples from the major manufacturers (GE, Siemens, Philips). It is very hard to support a specification without concrete examples. BIDS fields are often defined by experts that have a clear vision of what is needed, but there can be unintended consequences if the implementation is left to end users that have no idea what this data looks like.
Here's some data shared from when this was an issue: https://github.com/bids-standard/bids-specification/issues/1024#issuecomment-1059453132 That is from a tweaked version of the CMRR Siemens multiband sequence. At that point, this was still a bit hacky from the pulse sequence side of things so it's unclear if there would be a clear and consistent field to use. At least for now, it might be somethings uses might need to manually specify for after automated dicom conversion.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 87.88%. Comparing base (
01025da) to head (fc2f435).
:exclamation: Current head fc2f435 differs from pull request most recent head cd52516. Consider uploading reports for the commit cd52516 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## master #1451 +/- ##
==========================================
- Coverage 87.92% 87.88% -0.04%
==========================================
Files 16 16
Lines 1375 1354 -21
==========================================
- Hits 1209 1190 -19
+ Misses 166 164 -2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
As a separate file, should there be a key(s) that denotes whether a noRF scan is linked to another scan and, if they were collected as part of one acqusition, whether it was at the beginning or end of the scan.
I assumed that noRF scans would operate similarly to sbrefs, in that you wouldn't have one without an associated "primary" scan. One could also conceivably acquire an sbref at the beginning or end of the primary scan, or even in a separate scan, but we don't use metadata to explicitly encode that information. Is it necessary for noRF scans?
@tsalo can you provide exemplar DICOM validation datasets as sourcedata?
I also have some DICOMs from the CMRR sequence, with five echoes and magnitude + phase reconstruction. They're available at https://upenn.box.com/s/cgoc7851m3uo4yinxqxqlfv53em0n75b.
It is definitely possible to have one without a specific primary scan. For example, there are several studies that collect a single stand alone noise scan during a scanning session. Besides noRF it would have identical parameters to all the other bold runs in a session. One could arbitrarily link it to one of the bold runs, but that wouldn't be necessary.
Within the existing bids framework, I guess that would mean giving the noise scan a distinct run identifier and, if it's collected as part of a bold run it would have have the same run identifier. From my understanding, no changes to specs would be needed for this.
Since it might mess up stimulus timing later, it would be useful, but not essential to know if the noise scans were collected at the beginning or end of a run.
@handwerkerd in that case, maybe we should recommend using an IntendedFor field in the noRF's metadata.
I think events files should be adjusted as part of the BIDSification process, rather than within the BIDS dataset, but I'd like to get @effigies' thoughts on it.
I have little context for how these files are used, so my thoughts are more based on experiences with "associated data" files in BIDS, such as fieldmaps.
I think IntendedFor is one of my least favorite aspects of BIDS, as it encodes processing decisions within the raw data. I much prefer the B0FieldIdentifier approach to indicate which files can be used collectively to estimate a fieldmap, and then the task is to associate a fieldmap to a file. (The B0FieldSource metadata still encodes processing decisions in data, which I don't like, but at least they are associated in the BOLD file's metadata. I would prefer it if there were some standard way to map files in the bids-app protocol.)
If the idea here is to use the nearest noRF scan to aid in processing a given file, it seems like the simplest way to encode that is through the acq_time column in scans.tsv. The selection process is then part of the algorithm of the correcting tool. If you really need to encode intent, then I would suggest following the lead of B0FieldIdentifier and only use IntendedFor if you are unable.
Also: What files might be corrected by this, in addition to BOLD? Should we add it to DWI, ASL, anat, etc?
I much prefer the B0FieldIdentifier approach to indicate which files can be used collectively to estimate a fieldmap, and then the task is to associate a fieldmap to a file. (The B0FieldSource metadata still encodes processing decisions in data, which I don't like, but at least they are associated in the BOLD file's metadata. I would prefer it if there were some way to map files )
That sounds reasonable... maybe something like ThermalNoiseIdentifier and ThermalNoiseSource? @handwerkerd WDYT? I also agree that using the scan time can be a good heuristic for defining the field, as folks often do with field maps.
Also: What files might be corrected by this, in addition to BOLD? Should we add it to DWI, ASL, anat, etc?
I know it has been applied to DWI data at minimum, but I'm not sure about other modalities. It's meant as a measure of the thermal noise level, I believe, so I can imagine it being useful in other modalities. At least ones that typically involve some kind of denoising over time (e.g., ASL)... but I'm very new to this acquisition, so I could be wrong.
I think this might be getting overly complex (partially my fault). Any MRI sequence can be collected with noRF and this can be useful to measure the noise floor for any pulse sequence. In practice, you can tell exactly what it is is linked to because all the other parameters should match.
The only thing that makes this messy is that there is currently a single pulse sequence that collects some noise volumes a the end of an fMRI run so that a dataset will have multiple noise scans with each one collected as part of a specific run. The only reason we would need to know if it is from the beginning or end of a run is because BIDS requires actively cutting up the runs to separate the noRF from the normal volumes, which risks creating timing issues. As long as a script is included that documents what is done, this run order might be something that's not worth specifying in BIDS.
BIDS requires actively cutting up the runs to separate the noRF from the normal volumes
I'm not sure that's true, but I guess that would reopen discussion of https://github.com/bids-standard/bids-specification/issues/1024#issuecomment-1065415197, and this has apparently been open for a year, so I'll try to move forward and not look too much backward.
As long as a script is included that documents what is done, this run order might be something that's not worth specifying in BIDS.
This might be best until there are are enough such scans that it is worth developing a consensus on how to encode it. I think we could still suggest calculating the two split files' acq_times from the original BOLD file's acq_time and duration, as that does not require introducing any more new concepts to BIDS.
I noticed this is still sitting around unmerged since last March. @ericearl or @erdalkaraca Can one of you review?
sorry - coming late to discussion here. Isn't it can be for any sequence really, and thus can apply to other suffixes and thus it is more of having some _acq-noRF annotation than changing the target modality suffix?
@yarikoptic it's a bit of a judgment call, but I think noRF is more similar to sbref scans than a separate acq-noRF run. Here's why:
- The
noRFvolumes are typically (always?) acquired as part of the BOLD run. rather than as a separate scan. The main reason this proposal splits them into a separate file is because adding a new metadata field to index no-RF volumes in the time series would make things much harder for tools like fMRIPrep. - The
noRFvolumes are always associated with the run they're acquired with. You wouldn't acquire a separatenoRFrun and apply that to multiple BOLD runs. Given that association, we would need anIntendedForfield (or an equivalent toB0FieldSource/B0FieldIdentifier) if we usedacq-noRFinstead of a suffix. As it stands, we can rely on the implicit links we have due to overlapping entities, much like we have withsbreffiles at the moment. - The
noRFvolumes are not useful on their own.
@tsalo and @yarikoptic The current sequence I'm using (CMRR) has an option to tag noRF scans to a longer BOLD run, but, to my knowledge, this is the only common sequence where this is an option, and I can definitely point to studies where noRF scans were run on their own (see: https://www.sciencedirect.com/science/article/pii/S1053811910014503 ).
That said, what makes noRF interesting is that it can be used on any pulse sequence. That is, one can run a an EPI BOLD acquisition with RF off or an MPRAGE with RF off. In that context, noRF is always linked to whatever sequence would have been run if RF were on. In that context, it feels like a suffix to me.
As I grumbled above, the real issue here is the brittleness of the BIDS file naming structure compared to the range of things people do with MRI acquisitions. That's a harder problem to solve.
In relationship to this PR, my goal is to get something specified and merged and I'm fine with any of the currently proposed options.
Perhaps one compromise is to to allow noRF to either be a suffix if it's directly part of another sequence or an acq if it's a stand alone acquisition.
or an MPRAGE with RF off. In that context, noRF is always linked to whatever sequence would have been run if RF were on. In that context, it feels like a suffix to me.
so would not there be a conflict if you have multiple modalities (thus having suffixes e.g. T1w, T2w, ...) with noRF for each one of them -- how to discern what that _noRF for?
or an MPRAGE with RF off. In that context, noRF is always linked to whatever sequence would have been run if RF were on. In that context, it feels like a suffix to me.
so would not there be a conflict if you have multiple modalities (thus having suffixes e.g. T1w, T2w, ...) with noRF for each one of them -- how to discern what that
_noRFfor?
This is the problem with BIDS focusing on contrasts rather than pulse sequences. A noRF image would be neither T1w nor T2w. It would be an acquisition with no excitation (and no flip angle) thus the echo spacing between the non-existent excitation and readout is also meaningless. An EPI acquisition where the flip angle and echo times were tuned for T1w or T2w should generate similar noise profiles with noRF.
I understand that, but my point was - if you have sub-1_T1w.nii.gz and sub-1_T2w.nii.gz and for each one you get noRF -- how those two files are to be named?
if we are to with a suffix, I guess we could piggy back on mod and thus have them sub-1_mod-T1w_noRF.nii.gz and sub-1_mod-T2w_noRF.nii.gz
That looks fine to me (and I think it's what this PR would currently do?)
All this PR will do right now is make noRF suffix valid anywhere bold is valid. It does not provide for the mod-<label> entity that is included in the description, which seems like a problem. There will be no required metadata for this file. Is that okay?
I'll add the mod entity now. I see noRF files are similar to sbrefs in that they should generally rely on an implicit connection to the original file, so I can't think of any metadata they would need.
I changed the mod- examples to cbv and bold, but I can add noRF as a valid suffix for other MRI rulesets if folks want that.
Should I start the 5-day clock now that there are two approvals?
@bids-standard/raw-mri @bids-standard/raw-mri-func This is now open for voting and will be merged on Wednesday, April 24 if no one raises any major concerns.
In order to vote, please add a 👍 or 👎 reaction to this comment. If you vote 👎, it would be really helpful if you could post a follow-up comment explaining why.
Closing out the vote and merging.