afni_proc compatibility

[dowdlelt]

AFNI recently added the ability to use various combine modes with multiecho data in afni_proc.py. One of these involves selected a tedana.py version to use for the combination, rather than the included, older version. At the moment, pointing it at the tedana.py in the me-ica repo works perfectly. Will that compatibility remain as this version of tedana undergoes upgrades/updates? As it works now, AFNI prepares the zcat data, and a name for the TED folder and expects to find a dn_TS.nii at the end, so it would be relatively insensitive to internal changes (though that of course may change. )

[emdupre]

Hi @dowdlelt ! That's a great question. Yes, I think the medium-term plan is to retain compatibility with zcat'ed data, as well as supporting individual files for each pre-processed echo.

If that compatibility is eventually removed (since zcat was originally included as a convenience step for tedana), we'll work with the @nih-fmrif team to make sure that change is sync'ed with the AFNI interface. As you point out, though, that will be a breaking change for older versions of tedana, so I imagine it won't be for some time yet....

[dowdlelt]

Awesome, thanks for the update - the zcat step does add some data replication and storage overhead that it may be better off avoided...but there is plenty of time to solve that.

[handwerkerd]

I'm not part of the AFNI team, but I've been speaking to them a good bit about this and I can say that the way the AFNI wrapper code is written for this step is intentionally modular. If there comes a point where a different version of the denoising step requires different inputs, it should be very possible to collaborate to make sure the tedana wrapper in AFNI can handle this. It general, it would be nice to accelerate the demise of the creation of the zcat volume (which I think is mostly an artifact from the earliest version of MEICA when the ICA step was applied to a single volume containing all 3 echoes). I don't think any code in tedana still makes use of the fact that the 3 echo time series are merged into a single volume)

[afni-rickr]

Right now, the tedana_wrapper.py for afni_proc.py just does masking, scaling and zcat steps (no longer adding an additional mean, cutting betas in half). We would be happy to take out any or all of those steps.

Plus, we are not wedded to the tedana.py version distributed with AFNI (or even to distributing any version with AFNI). An advantage of distributing something is that it is a known entity to us. But if it would make more sense to upgrade it to your version, or even to stop distributing it at all, those are possibilities. There are not so many people (that I know of) who are using this extensively yet, with @dowdlelt possibly doing the most. It is a fine time to modernize our version, especially with him on board.

For me, it is nice to be able to give tedana.py:
- echoes, echo times, a mask, any extra tedana.py options from the user
and get back:
- OC, dn_OC, all components and partitioning (as we get now)

There are some capabilities that I want to add around tedana.py (projecting good components from bad, and applying the orthogonalized components, instead; processing before tedana.py (e.g. regression and censoring); using the ortvec with our OC method), so it is nice to have the listed outputs.

I would be happy to chat about any of that. The first and third capabilities (using projection) will hopefully be done this week.

[emdupre]

Thanks for the update, @afni-rickr ! It would be great to merge efforts on this.

From our end, there's really no reason to keep the zcat step, though we're ok to continue supporting it for now. Our reasoning was that if, for some reason, users wanted to be able to easily "roll back" to older versions, removing it would be a breaking change-- unless your wrapper will supply either the zcat or individual echos ?

The hope in this re-write is an independent python package (we're 4 system calls away !). Then you could pip install a set version. If you would prefer to distribute the underlying code supporting that version, that's also a possibility, though I imagine that keeping it up to date would be somewhat more difficult.

Looking forward to hearing your thoughts !

[afni-rickr]

Thanks for the comments @emdupre, and thanks for all of your work on the MEICA package!

It would be no problem to give our wrapper an option to zcat or not. Though for the most part, we do not expect people to directly use the wrapper. It is mostly a convenience tool to be called by afni_proc.py, so the user proc script does not get tooooo ugly.

Making your tool pip installable is a nice idea. Yes, it would surely be better if we did not distribute MEICA with AFNI.

I got the 2 updates that were mentioned yesterday in. Now afni_proc.py has methods to take either the AFNI OC or MEICA OC time series, and project from it the rejected components that have been orthogonalized to the accepted ones. I don't know if that is going astray from ICA, but it did not feel good to project terms that are correlated with ones that are considered "good" BOLD. It might be nice to offer such an option in your version of tedana.py. I would be happy to chat more about this. The user can choose which way to go in afni_proc.py.

While I am at it, may I request one little option? It would be nice to be able to specify a random number seed to tedana.py, just for regression testing. Seeding the random number generation should hopefully allow the same call (with the same software) to produce identical results. For example, sticking the following at the top of tedana.py leads to duplicate results across multiple calls:

import random
from scipy import random as numx_rand
random.seed(17)
numx_rand.seed(17)

Okay, that is enough babbling for now, thanks!

[emdupre]

Thanks @afni-rickr ! We actually have already implemented the set seed here— it's (partially) why we're able to get passing CI results now 😄

I'd love to hear more about the projection option ! Would you be willing to open an issue so we can focus the discussion there ?

Thanks so much !

[emdupre]

Sorry, I should clarify that our CI as it's currently written is a giant integration test. We're working to add unit tests next, but we wanted to ensure that results were consistent with the original MEICA script for as long as possible. This will likely change, though, after we implement #14, for example.

[CesarCaballeroGaudes]

I really support the option of projection considering the good components (a.k.a. non-aggressive denoising in other ICA-denoising algorithms such as FIX or ICA-AROMA). I had a quick chat about this with @afni-rickr while I was in NIH recently. In addition, it would be nice that the integration in afni_proc.py also returns the ME-ICA denoised datasets for all echoes. This is available in the the ME-ICA release 3.2. We are using these datasets to develop a multiecho version of 3dPFM.

[afni-rickr]

Great @emdupre, that's perfect about --seed. :) And it's good to know that you have so far kept consistency with the original MEICA. It will be fair to change to a new ICA.

Hi @CesarCaballeroGaudes! Yes, thanks for the description of the regression. @handwerkerd and I verified/duplicated it during the recent BrianHack with 3dDeconvolve. But now afni_proc.py has gone one step further to keep the good components in the data. @emdupre suggested I open a new issue for chatting about that. An intelligent person might open that issue first to reference it here. Too bad...

The '-combine_method tedana' does take the ME-ICA output from tedana.py. There are actually 7 -combine_method choices right now, with 'tedana' being the direct tedana.py result.

[jbteves]

I think it's worth rekindling discussion on this and adding some of the details discussed on calls, @emdupre. I believe we've brought it up a few times on conference calls, and the last time it was brought up, we decided that if we can do a hackathon it would be good to have the AfNI crew available if they think it's ready for wrapping. Please correct me if this is incorrect.

[afni-rickr]

Hi @jbteves . Note that we might even do another hackathon here late this year. Either way, I would be happy to wrap this in afni_proc.py. The trouble is simply getting to it (sorry @emdupre!). And I will write a reminder to my amnesic self to open an issue on that once actually starting.
For the record, I did add another afni_proc.py option to restrict the pure evil/tedort/orthogonalized regressors to those based only on "rejected", not including "midk_rejected", since there is less confidence in the midk terms being non-BOLD.

[jbteves]

This reminds me that based on the times @handwerkerd put for the Doodle, the hackathon should probably be mapped out. @emdupre WDYT?
Thank you for comment here, @afni-rickr, I'm sorry I didn't reply-- I apparently also suffer some amnesia, since I thought I had. It's no rush; this is part of my effort to make sure that there's a viable path forward for all issues. It seems that you are ready to wrap either when you have time or at the hackathon, correct? Would you mind letting us know if there's anything we can do to ease that process for you?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to tedana:tada: !

[tsalo]

I think perhaps we should break this issue into smaller chunks. Based on discussions had at the hackathon, I don't think there are any problems with the way we're structuring outputs, so any compatibility issues are probably matters of individual features AfNI folks would like to see in an AfNI-integrated version of tedana.

Are there any related issues we should open and are we ready to close this issue?

[afni-rickr]

Hi @tsalo, indeed afni_proc.py was able to apply tedana just before the hackathon. However, there were plans to output the components and details in a json format, and I will need to make some AFNI-side updates for that (hmmm, readthedocs mentions ica_decomposition.json).
There was also a discussion about whether some 'ortvec' outputs could be made, where some components were (optionally) orthogonalized with respect to others. But that's been working on our side for a while, and would only need updates for the json format.
So none of this really needs to kept as an open issue for tedana, unless you would like to use it to track/encourage progress on this end.

[stale]

This issue has been automatically marked as stale because it has not had any activity in 90 days. It will be closed in 600 days if no further activity occurs. Thank you for your contributions to tedana:tada: !

[handwerkerd]

I believe this issue is fully resolved now. AFNI has recently been updated to make it fully compatible with the current tedana. If you search for m_tedana in the afni_proc.py documentation you'll see the options that let you run AFNI using your own version of tedana rather than the original me-ica that's shipped with AFNI.

You can run @Install_APMULTI_Demo1_rest to download a dataset and scripts that demonstrate how to run scripts that use this tedana with AFNI.

Thank you @afni-rickr!