bids-standard / bids-specification

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

Request for comments on an implementation of BEP 16 #1909

Open mattcieslak opened 3 months ago

mattcieslak commented 3 months ago

@tsalo and I have been working on getting the derivatives of our dMRI post-processing workflows into BEP16 compliance. We'd appreciate some thoughts and comments on the file names we're producing. Personally, I think the entities and suffixes in BEP16 are intuitive and am pretty happy with how these look.

Here is a list of the outputs from a workflow that fits GQI and Tensor models in DSI Studio and DKI models from DIPY. It warps the scalar maps from these outputs into MNI2009cAsym space. (any thoughts @frankyeh, @arokem?)

And here are the files written out by a tensor+MAPMRI fit done in TORTOISE. (any thoughts @eurotomania?).

There are a few other files in this directory that list the expected outputs from our workflows, but they also list file formats that are not included in BEP16, such as the fib, mapping and the AMICO config pickle file. All the files have documentation but only those listed under Scalar Maps might be included in BEP 16.

Thanks in advance!

frankyeh commented 3 months ago

they look good to me!

On Tue, Aug 27, 2024 at 9:18 AM Matt Cieslak @.***> wrote:

@tsalo https://github.com/tsalo and I have been working on getting the derivatives of our dMRI post-processing workflows into BEP16 compliance. We'd appreciate some thoughts and comments on the file names we're producing. Personally, I think the entities and suffixes in BEP16 are intuitive and am pretty happy with how these look.

Here https://github.com/PennLINC/qsirecon/blob/main/qsirecon/tests/data/scalar_mapper_outputs.txt is a list of the outputs from a workflow that fits GQI and Tensor models in DSI Studio and DKI models from DIPY. It warps the scalar maps from these outputs into MNI2009cAsym space. (any thoughts @frankyeh https://github.com/frankyeh, @arokem https://github.com/arokem?)

And here https://github.com/PennLINC/qsirecon/blob/main/qsirecon/tests/data/tortoise_recon_outputs.txt are the files written out by a tensor+MAPMRI fit done in TORTOISE. (any thoughts @eurotomania https://github.com/eurotomania?).

There are a few other files in this directory that list the expected outputs from our workflows, but they also list file formats that are not included in BEP16, such as the fib, mapping and the AMICO config pickle file. All the files have documentation https://qsirecon.readthedocs.io/en/latest/builtin_workflows.html but only those listed under Scalar Maps might be included in BEP 16.

Thanks in advance!

— Reply to this email directly, view it on GitHub https://github.com/bids-standard/bids-specification/issues/1909, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACDI4PIWY7B7AK2F5CMWTDZTR4BDAVCNFSM6AAAAABNGD5CCKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ4DSMZUGM3DGMI . You are receiving this because you were mentioned.Message ID: @.***>

arokem commented 2 months ago

Also pinging @Lestropie, @francopestilli, @oesteban and @PeerHerholz in case they have comments about this implementation of BEP16 in software proposed by @mattcieslak and colleagues.

Lestropie commented 2 months ago
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-txx_dwimap.json
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-txx_dwimap.nii.gz
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-txy_dwimap.json
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-txy_dwimap.nii.gz
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-txz_dwimap.json
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-txz_dwimap.nii.gz
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-tyy_dwimap.json
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-tyy_dwimap.nii.gz
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-tyz_dwimap.json
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-tyz_dwimap.nii.gz
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-tzz_dwimap.json
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-tzz_dwimap.nii.gz

This should instead be:

derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-diffusivity_dwimap.json
derivatives/qsirecon-DSIStudio/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-MNI152NLin2009cAsym_model-tensor_param-diffusivity_dwimap.nii.gz

(same for the other output space) , with the requisite metadata in the JSON to indicate that the fourth dimension encodes coefficients of a symmetric rank-2 tensor.

The key insights here---which may feel initially unintuitive but I'm increasingly convinced is the right way to do it---are that:

  1. A 3D volume like FA can be interpreted individually; something like the XY component of the tensor cannot. You can only do something sensible with the latter if you have all coefficients of the tensor, and you know what convention was followed in deriving those coefficients (eg. whether they are realspace XYZ components or imagespace ijk components) so that you can properly interpret that tensor representation.

  2. "The tensor" is not a "parameter". What's being estimated is the diffusivity. The tensor is the mathematical model used to encode the anisotropy of that parameter.

derivatives/qsirecon-DIPYDKI

For pipeline derivatives, it is (I believe) RECOMMENDED in BIDS that any hyphen in the directory name separate the name of the pipeline from the "version" of that pipeline. However I believe that "version" should be eg. a software version number a la Semantic Versioning, not "I ran the pipeline in two different ways and here are the two different outputs". Don't think it's enforced, and you may have just named things in this way for the sake of presenting this example here, but it nevertheless jumps out at me.

The "software used" that necessitates "versioning" here, mostly in metadata but optionally in the directory name also, is ultimately qsiprep, not DIPY or DSIStudio. If you want to encode the fact that some dwimap images generated by qsiprep came from software dependency A and some came from software dependency B, I would probably suggest simply indicating that in the model description strings; robust encoding of such should probably be deferred to BIDS Provenance.

... they also list file formats that are not included in BEP16, such as the fib, mapping and the AMICO config pickle file.

PeerHerholz commented 2 months ago

Hi everyone,

thanks so much for this @mattcieslak and @tsalo, it already looks amazing.

Concerning the specific details of outputs and naming, I would refer to @Lestropie's points which IMHO reflect the latest discussions and implementation of BEP016.

A while ago, we also started working on a little tool to test the proposal, it's a BIDS-App called bids_bep16_conv. However, we restricted it to run DTI or CSD via dipy, mrtrix or fsl on BIDS derivative conform preprocessed dwi data. It would be cool to connect our efforts as we also have to address the points outlined by @Lestropie, ie versioning and metadata.

Best, Peer