bids-standard / bids-specification

Brain Imaging Data Structure (BIDS) Specification
https://bids-specification.readthedocs.io/
Creative Commons Attribution 4.0 International
281 stars 165 forks source link

the inheritance principle and BVEC files #1293

Open neurolabusc opened 2 years ago

neurolabusc commented 2 years ago

I think the BIDS documentation on the inheritance principle and sample OpenNeuro datasets may mislead many users. The bvec files are in image space, not world space. Therefore, they only apply to multiple images if both the vector tables AND image angulation are identical. Image angulation is often adjusted across scans, either by the user or due to auto-align. While some centers only acquire DWI data with the angulation aligned to the scanner bore (e.g. no angulation), this is not a consistent practice. The opportunity for unintended consequences is very high, especially if this caveat is not emphasized.

The FSL bvec format used by BIDS is described here and here. It will flip the first dimension of the imaging data if the data has a positive determinant, which makes it very unintuitive and can confuse people if the data is stored on disk in LAS versus RAS.

sappelhoff commented 2 years ago

I am not familiar with the topic, but perhaps @Lestropie can offer his opinion.

Lestropie commented 2 years ago

Hey Chris

I think the situation where inheritance is valid for specifically bvec is for vendors where the diffusion sensitisation gradients are themselves applied with respect to the image axes; from memory there's at least one that does that?

Agree that it's a fringe use case, and being permissive of such potentially introduces a risk of mis-use. But given other discussions around the Inheritance Principle, and the current state of conversion softwares, my suspicion is that basically nobody uses it anyway. The current solution is one of generality: regardless of .bvec or .tsv, just find the metadata file closest to the the data file of interest and use that. If you wanted to explicitly forbid inheritance of .bvec for DWIs, ie. force that any DWI NIfTI have a .bvec with identical filename except for the file extension, in order to protect against misuse, then:

  1. Would not only need to explicitly state such in the spec, but also either use that same behaviour for .tsv's, or provide justification as to why the operation of the inheritance Principle should differ between those two cases, despite both just being "tabular data".

  2. This would preclude cases where such inheritance absolutely makes sense. Eg. if there is an entity-linked file collection containing *_part-mag_dwi.nii and *_part-phase_dwi.nii, to me it makes sense that there should be one .bvec / .bval pair *_dwi.(bvec|bval), not two.

Another related point, this time in the context of derivatives. In most cases, due to bvec rotation according to subject motion, even if a single bvec file is applicable to the raw data for al subjects, subject-specific bvec files would be needed to store the derivatives. Conversely, I have fairly recently encountered a situation where a group-level analysis was predicated on q-space resampling having been performed in order for the set of q-space samples represented in the image data to be equivalent across all subjects; defining that set just once using the Inheritance Principle was therefore highly reflective of the nature of that dataset.

It will flip the first dimension of the imaging data if the data has a positive determinant ...

The determinant issue I raised in #243; probably need to make an attempt at that one myself at some point.

neurolabusc commented 2 years ago

@Lestropie different manufacturers record gradients differently. Specifically, GE gradients are in MR physics space (phase, frequency and slice direction), while Siemens and Philips vectors are relative to the scanner bore. BIDS adopts the FSL bvec format which is in image space (column, row, slice).

@effigies I would be tempted to at least add a caveat to the inheritance principle noting that sharing bvecs across images requires that all DWI images share the same angulation. A very smart BIDS validator could also check that this is the case.

I have already voiced my concerns over the principle and do think it will become harder to support as application sandboxing becomes more popular. However, it is now an accepted part of the format, so I do think we have a responsibility to flag the proper use in the documentation.

neurolabusc commented 2 years ago

As an aside, it could be I am more aware of this issue as we work with stroke patients where factors that include kyphosis means that participant anatomy is often not well aligned with the scanner's axis. It is possible that many studies of healthy adults that do not use auto-align functions are able to always acquire DWI scans without applying any angulation. In that case, the input bvec files would be common across files.

I certainly think it is possible to share input bvec files. I just think we need to make people aware that this is not always the case.

effigies commented 2 years ago

Just for some context, @rwblair did look through all of OpenNeuro last week. ds114 is the only one that uses the inheritance principle for bval/bvec. On the other hand, every single other case of bval/bvec in OpenNeuro had identical bvals/bvecs (as tested by checksum). So it does seem plausible that the clinical context is the difference here.

I agree that we should add some clarifying and/or warning text. I would be happy to review it, but I don't think I could generate it.

And is there a rule that could be reasonably applied on a per-file basis to check for the handedness issue? Or is the check you are proposing for the validator to check that the handedness of the affine is constant throughout dwi images in a dataset?

Lestropie commented 2 years ago

different manufacturers record gradients differently

I was speaking not with respect to the storage of the diffusion sensitisation information within the DICOM headers, but the actual application of the diffusion sensitisation gradients during acquisition. If the sequence on a platform were to read in the user's desired gradient table (in whatever format), but interpret that as sensitisation directions to be applied relative to the image axes rather than the scanner's physical space, then that would make the bvecs identical across subjects regardless of image FoV orientation (conversely the MRtrix-style gradient table---#349--- would be different between subjects). In that case it would be entirely reasonable for bvecs to be accessed via inheritance for images that have different affines.

I thought that there was one vendor that did this, but I may be wrong.

is there a rule that could be reasonably applied on a per-file basis to check for the handedness issue?

No. The handedness issue is something that is (previously silently) baked into the bvec format specification, which can result in an erroneous table if either:

But it just results in some transformation of the gradient directions; there's nothing "wrong" in terms of formatting. What happens instead is that estimated fibre orientations will be erroneously transformed. There is a published method and implementation that attempts to detect the presence of the problem, but it's far too computationally expensive to be included in a generic validation process.

Lestropie commented 2 years ago

Given the prospect of abolition of the inheritance principle has been again raised here, I'll take the opportunity to draw attention to an idea I had fairly recently, which is that the utilisation of the inheritance principle should be primarily programmatic:

https://github.com/Lestropie/bids-specification/issues/6

Most data generation, whether raw or derived, will be independent per data & metadata pair; or if not, independent per session; or if not, independent per subject. Therefore any use of inheritance at a level higher than the level at which those data were generated needs to be done explicitly (this even extends to the bvecs / bvals for magnitude / phase DWI data as mentioned above if using dcm2niix). My idea was that this should be achieved almost exclusively programmatically, running at the dataset level to identify redundant metadata and centralise it through inheritance, rather than having users maybe implement it manually and maybe get something wrong.

One could argue whether there's any point to have one piece of software implement inheritance at save and then API softwares implement inheritance at load time, but I still maintain that it's what is most faithful to the hierarchical nature of metadata and therefore such data are best stored in that fashion.

Again, only tangentially relevant to the Issue at hand, just thought I'd add in case it influences either a short-term spec change or a long-term agenda.