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

Request for comments on an implementation of BEP 16 #1909

Open
mattcieslak opened this issue Aug 27, 2024 · 4 comments
Open

Request for comments on an implementation of BEP 16 #1909

mattcieslak opened this issue Aug 27, 2024 · 4 comments
Labels

Comments

@mattcieslak
Copy link
Contributor

@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
Copy link

frankyeh commented Aug 27, 2024 via email

@arokem
Copy link
Collaborator

arokem commented Sep 3, 2024

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
Copy link
Collaborator

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.

  • BEP016 was deliberately reduced in scope to cover only voxel-wise diffusion models. Anything derived from tractography that results in a 3D or 4D image that can be adequately described with BEP016 metadata can be sensibly stored. But streamlines data are beyond BEP016. The unindexed BEP for tractography data is here. I still had outstanding work to do there in determining the right set of metadata to cover an adequate breadth of use cases from first principles, mostly based on the work I did for my chapter in this book.

  • Can you provide an example of an AMICO pickle file? My best guess is that it contains metadata relating to how the diffusion model is fit, which would be preferable to be stored as BIDS specification JSON metadata rather than a software-specific dump, in which case we could consider expansion of BEP016. I don't really like having specification-dictated metadata key names for every single possible model fitting parameter, as it makes what is supposed to be a data specification very strongly tied to specific software, but maybe there are some such parameters that can be reasonably generalised.

@PeerHerholz
Copy link
Member

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

@Remi-Gau Remi-Gau added the BEP label Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants