nipreps / mriqc

Automated Quality Control and visual reports for Quality Assessment of structural (T1w, T2w) and functional MRI of the brain
http://mriqc.readthedocs.io
Apache License 2.0
299 stars 132 forks source link

ENH: Adds diffusion-related IQMs. #1131

Closed arokem closed 7 months ago

arokem commented 1 year ago

I am putting up this branch so that folks at OHBM hackathon have an upstream branch to make PRs against, to add the different IQMs that we are all working on.

arokem commented 9 months ago

We would like to be able to group b-values by orders of magnitude. Is there already a function in the code-base that does something like that? Alternatively, we could copy over some code from DIPY:

https://github.com/dipy/dipy/blob/master/dipy/core/gradients.py#L816C1-L842C1 https://github.com/dipy/dipy/blob/master/dipy/core/gradients.py#L923-L950

The idea would be to calculate IQMs by groups, based on the b-value "tiers".

arokem commented 9 months ago

NVM - the previous comment is silly, because obviously we're going to need DIPY as a dependency for so much of this that it doesn't make sense to start copying over code.

oesteban commented 8 months ago

We would like to be able to group b-values by orders of magnitude. Is there already a function in the code-base that does something like that? Alternatively, we could copy over some code from DIPY:

https://github.com/dipy/dipy/blob/master/dipy/core/gradients.py#L816C1-L842C1 https://github.com/dipy/dipy/blob/master/dipy/core/gradients.py#L923-L950

The idea would be to calculate IQMs by groups, based on the b-value "tiers".

Sorry, I missed your comment. We actually do have this already in the workflow. It uses DIPY underneath but also is able to create b-value "clusters" for DSI, thanks to an algorithm @edickie first proposed in dMRIPrep.

arokem commented 8 months ago

Thanks @oesteban : can you point to this code? Might be useful here as well for DSI-like acquisitions?

oesteban commented 8 months ago

Thanks @oesteban : can you point to this code? Might be useful here as well for DSI-like acquisitions?

The interface definition is here: https://github.com/nipreps/mriqc/blob/a90222567759dfda097b792a22424d30d7cb9267/mriqc/interfaces/diffusion.py#L145

It provides several outputs: https://github.com/nipreps/mriqc/blob/a90222567759dfda097b792a22424d30d7cb9267/mriqc/interfaces/diffusion.py#L123-L142

I think you want a combination of the b_masks and b_values outputs (the former gives you binary masks over the b-vecs/b-vals table corresponding to each entry in the latter, which are ordered by growing magnitude).

This is in the workflow used here: https://github.com/nipreps/mriqc/blob/a90222567759dfda097b792a22424d30d7cb9267/mriqc/workflows/diffusion/base.py#L128

oesteban commented 8 months ago

@arokem I will be pushing this over the finish line during the next two days - apologies in advance for a potential deluge in commits :)

The plan is to connect your QC functions into the pipeline, so after merging, DWI will be generating and storing at the output - at least - the metrics coded in this PR.

oesteban commented 8 months ago

Thanks @oesteban : can you point to this code? Might be useful here as well for DSI-like acquisitions?

After a first dive into the code I can totally see where you will need this. I'll continue working on this for a little longer and let you know when it is ready for your review (see my last commit is marked WIP, I'd suggest waiting for me to finish this tomorrow before adding more commits from your side).

mattcieslak commented 8 months ago

Is there any interest in adding neighboring dwi correlation or fiber coherence from dsi studio?

arokem commented 8 months ago

Yes - I think so. These measures would be quite meaningful even without extensive processing, right?

mattcieslak commented 8 months ago

I think the raw_neighbor_corr was one of the best scoring features in hbn-pod2. It's calculated on the unprocessed data.

Also, I think the computation is simple enough that we could implement it in python without needing to add DSI Studio to the mriqc image. It would be a great way to confirm the actual formula of NDC too.

oesteban commented 8 months ago

@mattcieslak @arokem would reimplementing those metrics be a hefty lift? I say this because DSI Studio has commercial use limitations and we got rid of FSL just for that reason.

mattcieslak commented 8 months ago

@oesteban great point. The neighbor correlation method doesn't have a patent but might still have a non-commercial restriction on the use of the dsi studio implementation. I could implement the method from scratch based on the description in the paper, would that be ok?

oesteban commented 8 months ago

would that be ok?

That would be an outstanding contribution beyond MRIQC, useful for many. Perhaps DIPY would be a better home for the implementation, but I'd be happy to test it out here and port whenever it is finished -- that as @arokem feels it's best.

arokem commented 8 months ago

Yeah - agreed - I think that it would be really good to have here and also in general (and didn't realize it was calculated on unprocessed data! but even more useful here as such). And DIPY would be a good place for it. Maybe in a new sub-module in the denoise module? https://github.com/dipy/dipy/tree/master/dipy/denoise

oesteban commented 7 months ago

@arokem @teresamg this is at a point where it works locally on one DWI example. Considering the limited impact of the changes, I think this should work generally. Please review if you have the time and let's merge it so that we can start thinking of more metrics.

arokem commented 7 months ago

+1 to merge (I can't hit the "approve" button, because GitHub considers me the author of the PR).