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

[ENH] Use TSV for DWI gradients, and allow both voxel- and world- coordinates conventions #352

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,8 @@ distinguish different sets of phase-encoding directions.
**Combining multi- and single-band acquisitions**.
The single-band reference image MAY be stored with suffix `sbref` (for example,
`dwi/sub-control01_sbref.nii[.gz]`) as long as the image has no corresponding
[gradient information (`[*_]dwi.bval` and `[*_]dwi.bvec` sidecar files)](#required-gradient-orientation-information)
[gradient information](#required-gradient-strength-and-orientation-information)
to be stored.

Otherwise, if some gradient information is associated to the single-band diffusion
image and a multi-band diffusion image also exists, the `acq-<label>` key/value pair
MUST be used to distinguish both images.
Expand All @@ -522,21 +521,78 @@ In such a case, two files could have the following names:
The user is free to choose any other label than `singleband` and
`multiband`, as long as they are consistent across subjects and sessions.

### REQUIRED gradient orientation information

The REQUIRED gradient orientation information corresponding to a DWI acquisition
MUST be stored using `[*_]dwi.bval` and `[*_]dwi.bvec` pairs of files.
The `[*_]dwi.bval` and `[*_]dwi.bvec` files MAY be saved on any level of the directory structure
### REQUIRED gradient strength and orientation information

The gradient strength and orientation information corresponding to a DWI acquisition
is REQUIRED, and therefore, MUST be stored in the structure.
The gradient strength and orientation information stored in the structure MUST correspond
to "*effective*" values (as in, the actual orientations and strengths applied by the scanner),
rather than the "*requested*" values (as in, the original gradient table design that is
configured into the scanner's MR protocol settings).

The RECOMMENDED way to store gradient information is a `[*_]dwi.tsv` file corresponding
to one or more DWI files.
The format of this TSV file, and associated metadata, are described below.
The gradient information MAY be stored using the `[*_]dwi.bval` and `[*_]dwi.bvec` pair
of files.
However, the `[*_]dwi.[bval|bvec]` files are DEPRECATED and will be
phased out in a future release.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is a breaking change shouldn't this be under the 2.x consideration? if not perhaps the wording at present could be changed to support both the current required bval/bvec and add the tsv file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see this PR more as a bugfix (and hence that takes precedence over the backward compatibility principle). Happy to see proposals on better wording though.

That said, with the current text both would be supported. TSV would take precedence over bval/bvec, and implicitly (but not explicitly), the tsv should be equivalent to bval/bvec. The only "breaking" change is lifting the mandate to have bvec/bval files (if you have the tsv)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@satra did my reply address your concern? How would you address this issue?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry too many things going on. my suggestion would be to continue to require bval and bvec till bids 2.0. regarding it being a bug. is it really a bug, or just that it forces a certain consideration that does not work in all situations. i.e. software have to be aware of the details of the spec. if the latter, that to me is not a bug but an alienation problem, which can be addressed by making the tsv required/recommended moving forward. it's possible i'm not completely understanding the bug part of the spec.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@oesteban I am inclined on giving precedence to bvecs/bvals reason being that if we give precedence to TSV only a very narrow set of mainstream toolboxes will be able to handle BIDS DWI. @arokem @jchoude

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW - how does this PR breaks backward compatibility? The deprecation is exactly to tolerate the utilization of the old feature while issuing a strong recommendation of the replacement (https://en.wikipedia.org/wiki/Deprecation) - precisely to avoid breaking changes.

I'd agree with removing the .bvec/.bval only the 2.0 release (which would indeed be the breaking change).

Copy link
Collaborator

Choose a reason for hiding this comment

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

a breaking change in the specification is one where existing tooling stops working as a result of the change. typically deletions of required elements in any schema are breaking changes. additions are also breaking changes if the new keys introduced are required.

Copy link
Collaborator

Choose a reason for hiding this comment

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

a breaking change in the specification is one where existing tooling stops working as a result of the change.

Yup, and since .bval/.bvec will still be allowed, a dataset that currently works will continue working on the same tool. The requirement is to have at least one of the two options (.tsv or .bval/.bvec) - so if the .tsv is not present, the specification remains exactly the same.
The new format is RECOMMENDED, and will only be mandatory when the deprecation finishes. Possibly BIDS 2.0.

The only effect of this PR is that now a BIDS dataset can be written with a much better, unambiguous format for the gradients. And that's not breaking compatibility because scanner coordinates could not be encoded (without conversion to voxels) before.

Anyhow, besides I personally think this is not breaking compatibility, there's also the fact that we indeed discussed how this deprecation would happen when Matt first submitted the PR. It would be unfortunate that we revisited those decisions (for the reasons above, and because that will set some precedent as to maybe preempt future contributions).

My impression is that although this proposal has important substance changes (I particularly like @jchoude's summary above), we seem to be focusing more on the formal aspect of the deprecation (which has largely been discussed already).

Copy link
Collaborator

Choose a reason for hiding this comment

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

i completely agree that this sub discussion has to do with principles of BIDS rather than the substance of this PR. however, if this PR is merged software that worked prior to this could stop working. for example, wouldn't this mean that after this change none of the scripts using the FSL tools will work on new datasets unless someone did an appropriate conversion?

most software doesn't check on bids version for a dataset given that bids has not released 2.0. i think that's the fundamental question in this thread. this PR just happens to be an instance of something that brings this up. i don't want to hold up this PR, but it seems that there are other instances where such an issue could come into play. as bids expands this could happen at a modality specific level such as this, or the common principles level (there were a few inheritance issues that were pushed off to bids 2.0).

thus this is a meta discussion for bids as a whole, not just this PR. but merging this PR, means we have accepted this route at least for one instance.

Copy link
Collaborator

@oesteban oesteban Apr 29, 2021

Choose a reason for hiding this comment

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

for example, wouldn't this mean that after this change none of the scripts using the FSL tools will work on new datasets unless someone did an appropriate conversion?

For existing datasets, not at all - this PR does not mandate the update from bvecs/bvals to tsv.
For future datasets, you can generate the bvecs/bvals from the tsv file. Making this step more explicit will make it harder for the user, but will also ensure data is preserved as it was generated by the scanner (in the tsv file).

If someone is motivated and has the knowledge to implement a test within the BIDS validator to ensure that the tsv and bvec/bval are consistent, that would be really helpful for the DWI community.

The TSV file format is OPTIONAL, and will replace `[*_]dwi.[bval|bvec]` as the
default method for representing gradients.

The `[*_]dwi.[bval|bvec|tsv]` files MAY be saved on any level of the directory structure
and thus define those values for all sessions and/or subjects in one place (see
[the inheritance principle](../02-common-principles.md#the-inheritance-principle)).

As an exception to the [common principles](../02-common-principles.md#definitions)
that parameters are constant across runs, the gradient table information (stored
within the `[*_]dwi.bval` and `[*_]dwi.bvec` files) MAY change across DWI runs.
within `[*_]dwi.[bval|bvec|tsv]` files) MAY change across DWI runs.

**Gradient table specification (`_dwi.tsv` file)**.
Gradient strength and orientation can be provided in a TSV file named, for example,
`sub-01_acq-singleband_dwi.tsv` or `sub-01_acq-multiband_dwi.tsv`.
This TSV file must contain four columns, where the first three columns correspond to the
three components of a unit-norm vector determining one gradient orientation (also called "*b*-vector")
and the last column contains the gradient strength ("*b*-value") in s/mm<sup>2</sup>.
Each row of the TSV file corresponds to one sampled DWI, and therefore, the number of rows
MUST match the number of volumes in the corresponding DWI file(s).
In other words, the number of rows in the TSV file and the size of the last dimension of the
data array in the corresponding DWI file(s) MUST be the same.

Rows corresponding to *b = 0* s/mm<sup>2</sup> strengths MUST contain four zeros.
Correspondingly, the components of the *b*-vector associated with a nonzero gradient strength
(*b* &#62; 0) MUST represent a unit-norm vector.

Scanner devices may store the *b*-vectors with reference to the acquired voxel or the
scanner coordinate system (see [Coordinate systems](../99-appendices/08-coordinate-systems.md)).
The coordinate system specification is REQUIRED, and MUST be encoded by the column headers
of the TSV file.
Therefore, if the *b*-vectors in the gradient table are given in *voxel coordinates*,
the column headers MUST be `I`, `J`, `K`, `b`.
We will refer to this format as *IJK+b* in the following.
Alternatively, if the *b*-vectors in the gradient table are given in *scanner coordinates*,
the column headers MUST be `R`, `A`, `S`, `b`.
We will refer to this format as *RAS+b* in the following.

It is RECOMMENDED to store the gradient table information in the original coordinate system
stored by the scanner (therefore, unchanged).

It is RECOMMENDED to maintain the column ordering as described above (thus, either `I J K b`
or `R A S b`).

Example of `_dwi.tsv` file, following the *RAS+b* convention:

```Text
R A S b
0 0 0 0
1.0 0 0 1000
-0.0509541 0.0617551 -0.99679 1000
```

**Gradient orientation file formats**.
**Legacy `.bvec` & `.bval` specification of gradient information**.
The `[*_]dwi.bval` and `[*_]dwi.bvec` files MUST follow the
[FSL format](https://fsl.fmrib.ox.ac.uk/fsl/fslwiki/FDT/UserGuide#DTIFIT):
[FSL format](https://fsl.fmrib.ox.ac.uk/fsl/fslwiki/FDT/UserGuide#DTIFIT).

The `[*_]dwi.bvec` file contains 3 rows with *N* space-delimited floating-point numbers
(corresponding to the *N* volumes in the corresponding NIfTI file.)
The first row contains the *x* elements, the second row contains the *y* elements and
Expand Down Expand Up @@ -645,18 +701,35 @@ sub-<label>/[ses-<label>/] # MultipartID

### Other RECOMMENDED metadata

#### Phase-encoding information (common metadata fields)

The `PhaseEncodingDirection` and `TotalReadoutTime` metadata
fields are RECOMMENDED to enable the correction of geometrical distortions
with [fieldmap information](#fieldmap-data).
See [Common metadata fields](#common-metadata-fields) for a list of
additional terms that can be included in the corresponding JSON file.

#### Pulse gradient timing information

Although the *b*-value is the most common way to convey gradient strength, it does not
unambiguously depict the sampled coordinate in *q*-space.
The relationship between each *b*-vector and its corresponding *q*-space coordinate
can be calculated with the `BigDelta` and `SmallDelta` metadata fields in the corresponding
JSON file.

| Field name | Definition |
| :------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| SmallDelta | RECOMMENDED. Acquisition pulse separation time (in seconds). For Siemens, this is in the shadow header (0029,1120) under `sSpecPara.dSpecLipidSupprBandwidth` in milliseconds. This value is necessary for estimating ensemble average propagators in physical units. |
| BigDelta | RECOMMENDED. Acquisition pulse separation time (in seconds). For Siemens, this is in the shadow header (0029,1120) under `sSpecPara.dSpecLipidSupprDeltaPos` in milliseconds. This value is also necessary for estimating ensemble average propagators in physical units. |

JSON example:

```JSON
{
"PhaseEncodingDirection": "j-",
"TotalReadoutTime": 0.095
"TotalReadoutTime": 0.095,
"BigDelta": 0.005,
"SmallDelta": 0.0005
oesteban marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +771 to +772
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • Given that these appear in a general sidecar metadata space rather than a DWI-specific context, the names "BigDelta" and "SmallDelta" may not be specific enough.

  • If revisiting, it would be worth considering whether it is worthwhile adding these, or instead deferring more advanced diffusion encoding information entirely to the aDWI extension.

}
```

Expand Down
1 change: 1 addition & 0 deletions src/schema/datatypes/dwi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- .nii.gz
- .nii
- .json
- .tsv
- .bvec
- .bval
entities:
Expand Down