[ENH] Update DWI suffixes to include most common scanner derivatives

[Lestropie]

This PR is intended to supersede #1831 with additional content, as well as resolve #1862.

Rather than incrementing the spec with different aspects of scanner-generated DWI derivatives, as happened previously with #1725 and now #1831, it makes more sense to me to establish what that set is, and resolve the specification against the entirety of that set.

I have additionally created an exemplar dataset that shows all of the derivatives that can be generated by the product diffusion sequence on the Siemens XA platform:
bids-standard/bids-examples#452
If anybody knows of other derivatives that can be generated by other sequences / platforms, it would make sense to add those as extra subjects within that dataset.

While the sequence exports can include data encoding the outcomes of the tensor fit itself, those are exported in a proprietary format, cannot be trivially converted to NIfTI, and are therefore excluded from this list.

---

Some points requiring discussion (hence draft PR):

• The "exponential ADC" map is explained here. I've confirmed that if you compute the ratio of the trace-weighted image to the mean b=0 image you get the same result. This is the name that parameter is given in the protocol editor on the scanner console, but it's not a unique name; it can also be called "attenuation coefficient" or "attenuation factor". Most likely "expADC" is the most sensible suffix, but requires agreement.
• I've de-capitalised the existing TRACE suffix, and set these new suffices, based on my thinking registered over on bids-2-devel where capitalisation should apply to acronyms.
• I've not confirmed compatibility between the proposed exemplar dataset and the proposed changes to the spec. I presume this can be done, just not something I've done before.
  Edit: New dataset "dwi_deriv" for scanner-generated DWI derivatives bids-examples#452 (comment)
• One of the scanner-generated derivatives is labelled as "TENSOR_B0". This is not precisely equivalent to the mean b=0 image; rather, it is the estimated b=0 signal intensity based on the tensor model fit ("S0" is more commonly used for this term in diffusion model space). So it's just a T2-weighted image, but it's arisen from a tensor model fit. Therefore in the exemplar dataset, I've named these data "*_desc-tensor_T2w.*". I presume this is not currently permissible; I would personally prefer this kind of encoding as opposed to giving it its own suffix, but if others dislike it we'll need to figure out some different encoding.
  Edit: Converged on re-use of existing _S0map.

[effigies]

Overall these changes look good to me. Made some comments over on bids-standard/bids-examples#452.

[effigies]

My takes (as someone who's never analyzed DWI data):

• expADC: Seems fine.
• TRACE -> trace: Seems fine, as long as it happens before 1.10.0 is released (which is why I'm bumping this issue).
• desc-tensor_T2w: As noted on New dataset "dwi_deriv" for scanner-generated DWI derivatives bids-examples#452, desc-<label> is not permitted for raw files. The T2w suffix could be added to the DWI file rule, if people agree. dwi/sub-01_T2w.nii would probably be indicative enough that it's a scanner-derived T2w image, so an additional required entity would not be needed (although rec-tensor might be a sensible convention). I also don't see a problem with introducing S0 if people would be more used to calling those images by that name.
• Spec/example comparison: The desc-tensor_T2w is the only obvious discrepancy. See below for testing.

If you want to test with schema validation (you need Python 3.8+ and Deno installed):

# In spec directory
python -m venv .venv
source .venv/bin/activate
pip install -e tools/schemacode
bst export > src/schema.json
BIDS_SCHEMA=$PWD/src/schema.json deno run -A https://github.com/bids-standard/bids-validator/raw/master/bids-validator/src/bids-validator.ts /path/to/bids-examples/example

Note that you can add --reload to deno run to force Javascript and JSON files to be dropped from the cache. When fixes don't fix things, this is often a problem. And using -e in the pip install ensures that any changes you make to the schema will be immediately available to the next call to bst.

@CPernet @jhlegarreta @arokem As participants in the prior discussion (#1831) would you mind weighing in on the discussion points?

[jhlegarreta]

• expADC: I am OK with the term for the exponential ADC map. I see that "attentuation coefficient" and "attenuation factor" are mentioned in the docs where expADC is defined, so I'd say that it is enough.
• TRACE -> trace : definitely OK.
• desc-tensor_T2w / dwi/sub-01_T2w : I am fine with the latter as long as it is clearly explained. Not sure how rec-tensor would be used (i.e. dwi/sub-01_rec-tensor ?); same goes for S0: would it be like dwi/sub-01_S0 ? In either case, I am less inclined towards introducing a new label (either rec-tensor or S0) for this, but my view on the breadth of the extension is also limited.

Thanks for doing all this.

[effigies]

@jhlegarreta

Not sure how rec-tensor would be used

sub-01/dwi/sub-01_rec-tensor_T2w.nii.gz

rec-<label> is already permitted for most (if not all) MR images to indicate reconstruction method.

same goes for S0: would it be like dwi/sub-01_S0

Yes: sub-01/dwi/sub-01_S0.nii.gz

[jhlegarreta]

OK. Thanks for the explanation Chris.

Then sub-01_rec-tensor_T2w.nii.gz is more self-descriptive than sub-01_T2w.nii.gz to me, so my preference goes for sub-01_rec-tensor_T2w.nii.gz.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.23%. Comparing base (77ad779) to head (97d6a94).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1864   +/-   ##
=======================================
  Coverage   87.23%   87.23%           
=======================================
  Files          16       16           
  Lines        1410     1410           
=======================================
  Hits         1230     1230           
  Misses        180      180           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Lestropie]

After some time, I think I'm gravitating toward the _S0 suffix:

• While such images will almost always be T2-weighted, they're not acquired with the intention of obtaining T2-weighted contrast between different tissues; it just happens to be a spin echo sequence with a moderate TE.
• It would achieve consistency w.r.t. "here's the dedicated suffices to use for scanner-generated DWI derivatives".

[jhlegarreta]

Re #1864 (comment) Sound reasonable Robert. Thanks.

[Lestropie]

Was not aware of the existing suffix S0map. I think that generalising that suffix to represent "the estimated signal intensity were the contrast of interest to be absent", such that it applies to both TE=0 for fMRI and b=0 for DWI, may be the right approach?

[Lestropie]

Note that 7241c2b a4d10f1 also changes the definition of the "S0map" suffix:

< display_name: Observed signal amplitude (S0) image
> display_name: Projected baseline signal amplitude (S0) image

This is not just for compatibility with the augmentation proposed here. I consider that description to be erroneous for its existing usage in multi-echo fMRI. Because there, the signal intensity at TE=0 is not "observed"; it is reconstructed by fitting a mathematical model to the empirical data.

I have also updated the corresponding example dataset via bids-standard/bids-examples@2f23b50. But I'm yet to attempt the validation with the proposed schema update locally.

[effigies]

This LGTM. Small question about the entities.

[jhlegarreta]

So S0map seems a reasonable choice then 👍.

Do not know enough about fMRI data yet, so cannot comment on the rest of the points made. Also not familiar about how different scanners consider different phase encoding acquisitions 😬.

[jhlegarreta]

Projected -> Predicted? Estimated?

[Lestropie]

Don't feel particularly strongly for any choice.

[effigies]

This bikeshed can be repainted without backwards compatibility issues. If everybody's happy with the suffixes, then let's move forward.

[jhlegarreta]

Agreed. I'm fine if others feel like "projected" is the right.

[effigies]

This PR needs to be completed ASAP to be included in 1.10.0, or we need to back out #1725 to avoid releasing something we intend to change.

[jhlegarreta]

The PR is still marked as draft, but approving @effigies.

[tsalo]

I discussed this with @mattcieslak and we're both good with it.

[effigies]

In the absence of objection and with three approving reviews, I'm inclined to merge this next week. @Lestropie Given that you hadn't moved it out of draft, I want to make sure you've got time to respond if there were still open issues, to your mind.

[Lestropie]

I'm happy to proceed content-wise, my only concern was the absence of confirmation of validator compatibility, which I wasn't successful at myself; sorry I haven't been monitoring updates here or over at bids-standard/bids-examples#452. As long as that check produces sensible results I suggest merging both at the same time; I've flagged bids-standard/bids-examples#452 as being ready for review also.