ME-ICA / aroma

ICA-AROMA, as a Python package. A work in progress.
Apache License 2.0
7 stars 11 forks source link

Drop support for native-to-standard space transforms #11

Open tsalo opened 3 years ago

tsalo commented 3 years ago

Summary

This stems from https://github.com/Brainhack-Donostia/ica-aroma-org/pull/41#issuecomment-723421163. We will drop support for native-to-standard space transforms for now, and all native-space data must be accompanied with tissue probability maps for now.

Next Steps

  1. Remove all transforms.
  2. Drop FEAT directory support too, maybe?
tsalo commented 3 years ago

I think the key problem here is that nitransforms doesn't support nonlinear warps. Or at least I can't find any information on warps in their documentation.

CesarCaballeroGaudes commented 3 years ago

Can we ask @oesteban?

oesteban commented 3 years ago

I think the key problem here is that nitransforms doesn't support nonlinear warps. Or at least I can't find any information on warps in their documentation.

Nonlinear warps from FSL?

tsalo commented 3 years ago

@oesteban I think FSL is the most likely source of transforms (since ICA-AROMA is designed for FSL-processed data), but we are hoping to support data coming from anywhere. It doesn't sound like that's possible yet, though.

We could try to support FSL if and when poldracklab/nitransforms#51 is merged, though, and add support for each package as it is incorporated.

oesteban commented 3 years ago

@oesteban I think FSL is the most likely source of transforms (since ICA-AROMA is designed for FSL-processed data), but we are hoping to support data coming from anywhere. It doesn't sound like that's possible yet, though.

Not for fMRIPrep - I know this is not the only pipeline capable of running ICA-AROMA but probably the best test-bed for a standalone implementation of AROMA.

For the time being, c3d converts FSL distortion maps into ITK's.

eurunuela commented 3 years ago

Okay, so shall we drop all these dependencies and use probability maps until poldracklab/nitransforms#51 is merged?

oesteban commented 3 years ago

I think that was @tsalo's original proposal - do not bother at this point about how the TPMs are generated in the native space, just assume they are defined there.

eurunuela commented 3 years ago

I think that was @tsalo's original proposal - do not bother at this point about how the TPMs are generated in the native space, just assume they are defined there.

Yes. I wanted to make sure. I was starting to get lost in the conversation having now two different repos after the Brainhack Donostia tutorial 😅