Open CPernet opened 1 month ago
If wanting to add support for scanner-generated DWI derivatives in this way, is it worth trying up-front to cover all such derivatives, rather than doing it incrementally? In addition to ADC, TRACE, and now here FA, I've also seen exponential ADC, coloured FA, and tensor.
I agree that since we support scanner derived images, this should be all and not a subpart of them.
@Lestropie what suffix do you propose:_expADC
, _colFA
, _tensor
?
Had a quick look at what data I could find lying around:
SeriesDescriptionExtra
.SeriesDescriptionExtra
of "TRACEW
" rather than "TRACE
". Not that there needs to be a perfect correspondence between scanner sequence code and BIDS suffices._TRACEW
", I always assumed that these were trace-weighted images, ie. the spherical mean intensity of each unique b-value, not the trace of the tensor. In the only data I could find at hand the image data make little sense so I can't actually distinguish between the two visually. Either way I think stronger disambiguation is warranted.TRACEW
" series I have at hand are 4D with two volumes. Again, unclear what's actually being encoded here as I'm not convinced the data I have at hand make sense one way or the other.*_ColFA
" from the scanner; "_colFA
" would be fine I think._TENSOR
" series I've downloaded I can't convert to image data. I've no idea how these are encoded. _TENSOR_B0
" series, presumably the estimated signal at b=0 from the tensor fit. This is ultimately a _T2w
image, just the result of derivative calculations. Hard to know how to encode this trivially given the weird provenance.RE the difficulty in description of "scanner-generated derivatives" in #1725, something just occurred to me that evaded my mind in #1723. DICOM ImageType
makes a distinction between "original" and "derived". If the long term plan is to encapsulate within "BIDS raw" myriad images that are not the direct result of image data acquisition and reconstruction but downstream calculations from such, that's the reference terminology. Could maybe try to be diligent to use specifically "scanner-derived" to disambiguate from "BIDS derivatives"? This distinction will become more important once something like BEP016 progresses, given that scanner-derived and pipeline-derived images may encode the exact same parameter yet demand different naming conventions.
@Lestropie
_TRACE
, just adding things herecolFA
is the description ok? also changed the table/MACRO as per @effigies ScannerDerivatives
I did not change _TRACE, just adding things here
Sorry, should have more explicitly sub-divided my comments. Fully realize that this particular observation is outside the scope of this PR. But nevertheless think it's sub-optimal in #1725; and if this PR is expanded in scope to be "resolution of #1725 to how we think all DWI scanner-derived images should be handled", then given how recently #1725 was merged and the fact it's not yet part of a tag, I'm advocating to change it. Further, if there's a mismatch between what is encoded in such images and the current spec description, that needs to be resolved urgently.
As I understand it, there are no expFA, TENSOR images - correct? (never seen any)
Exponentiated ADC I think I've only seen offered on the console interface on Siemens XA, not VE. The TENSOR
DICOM series I've definitely seen, I just don't know what's encoded in it; neither dcm2niix
nor MRtrix3 mrconvert
knew what to do with it. I might try to collect a very short protocol on XA using the product sequence and export everything offered. Ideally bids-examples should have such a dataset; and creating that in conjunction with any relevant metadata encapsulates both FA and the already merged ADC & TRACE.
if happy with current changes, can you approve the merge?
I pushed a small fix for failing tests. Note I did not include my suggested fixes above.
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88.04%. Comparing base (
db6b07d
) to head (081a59f
).:exclamation: Current head 081a59f differs from pull request most recent head 2dd88b6
Please upload reports for the commit 2dd88b6 to get more accurate results.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Also committed your changes now. I think this is good to go?
Thx @Lestropie for comments @effigies for fixing the 'code'
Last Friday I collected some data from which I intend to generate some exemplar data over at https://github.com/bids-standard/bids-examples. Do you want to hold off until then?
+1 to @lestropie’s comment. If the scanners tend to call it “colFA” it’s better to leave it like that than use “DECFA”
On Mon, Jun 24, 2024 at 1:19 PM Robert Smith @.***> wrote:
@.**** commented on this pull request.
In src/schema/objects/suffixes.yaml https://github.com/bids-standard/bids-specification/pull/1831#discussion_r1650313452 :
@@ -42,11 +42,23 @@ DIC: display_name: Differential interference contrast microscopy description: | Differential interference contrast microscopy imaging data +colFA:
- value: colFA
- display_name: Colored Fractional Anisotropy image
- description: |
- Diffusion images reflecting the directionality of the diffusion tensor color-coded in red for
- left-right oriented fibers, in blue for superior-inferior oriented fibers and green for
- anterior-posterior oriented fibers.
I don't think BEP016 ever uses "DECFA" specifically. The fact that the nature of orientation encoding across volumes is directionally-encoded colour, and that the quantitative parameter encoded in the image is FA, are treated very deliberately as two separate dimensions. The distinction between scanner-generated derivatives and pipeline-generated BIDS Derivatives nevertheless means that there doesn't actually need to be correspondence between the two. Indeed a colour FA image generated by a pipeline will not use this suffix, even though the information encoded is identical.
I maybe have a slight preference for "colFA" just because it's consistent with data being emitted by many scanners that are intended to be encoded in this way.
— Reply to this email directly, view it on GitHub https://github.com/bids-standard/bids-specification/pull/1831#discussion_r1650313452, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA46NX7LJ6MYEDXNIFGT7LZI6M6JAVCNFSM6AAAAABIFE5ZMSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMZUGU4DQNRYGA . You are receiving this because your review was requested.Message ID: @.***>
fa, colfa, trace and adc updated
builds upon @effigies last commit on DWI https://github.com/bids-standard/bids-specification/commit/1d929e587775fa09489f95c94ffc0470b76020eb -- adding the FA (since scanners generate TRACE, ADC and FA)
I'm not expert in diffusion, but seems logical thing to do