tedana
tedana copied to clipboard
[DOC] Add documentation page on denoising approaches
Closes None, but stems from #819.
Changes proposed in this pull request:
- Add page on denoising strategies. I still need to figure out how to better integrate it into our documentation.
- Add sphinx-inline-tabs to documentation requirements.
- Add missing carpet plot (closes #803).
To do:
- [x] Figure out where to put the new page/information in our documentation.
- [ ] Add AFNI code to do the denoising.
- [ ] Discuss when someone would want to use each approach and why.
Check it out: https://tedana--823.org.readthedocs.build/en/823/denoising.html
Codecov Report
Base: 93.30% // Head: 93.30% // No change to project coverage :thumbsup:
Coverage data is based on head (
33ff086
) compared to base (d85dac3
). Patch has no changes to coverable lines.
Additional details and impacted files
@@ Coverage Diff @@
## main #823 +/- ##
=======================================
Coverage 93.30% 93.30%
=======================================
Files 28 28
Lines 2345 2345
=======================================
Hits 2188 2188
Misses 157 157
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
I really like the idea of adding several example preprocessing pipelines and noise regression pipelines to the documentation. I think the question of how to connect this information to the documentation and what to include on this page are linked. In the current text, these examples are linked with an explanation of aggressive vs non-aggressive denoising. Would you consider calling this something like "Using tedana output with other programs"? Then the focus will be how to use the output (including for aggressive vs non-aggressive denoising). It would also then fit as a link off off "Outputs of tedana"
Other general comments:
- You're saying
tedana
is doing non-aggressive denoising, but by your definition, I think it's aggressive. See: https://github.com/ME-ICA/tedana/blob/f1e98cb1630ebdfd6ba044ea16e4c06f4a3830df/tedana/io.py#L378-L379 The rejected components are fit to the data without regard to the regressors of interest. - Looking at the python code sample I think you're trying to show how regress out things like motion in addition to the tedana rejected time series, but you don't clearly say that anywhere.
- Perhaps this can be reorganized to show how to load the ICA mixing matrix and component table into python/AFNI/FSL and then regress out those data in isolation or in combination with other noise regressors. You'd still cover what you're calling aggressive vs non-aggressive, but focusing on that distinction is making the main use of these examples a bit more confusing.
- Why are we including FSL if FSL currently is currently so multi-echo unfriendly? If someone knows how to load the time series and regress them out in FSL, I'm fine including that example, but I don't feel like re-learning enough of FSL to get that working, and I'm hesitant to present a use case that none of the current tedana developers are using/supporting.
In the current text, these examples are linked with an explanation of aggressive vs non-aggressive denoising. Would you consider calling this something like "Using tedana output with other programs"? Then the focus will be how to use the output (including for aggressive vs non-aggressive denoising). It would also then fit as a link off off "Outputs of tedana"
To be honest, part of my goal was to document the differences in a general sense, so that AROMA users (or whomever) could read our documentation and understand what their own outputs mean. I'm hoping to link to whatever page we add this to in the AROMA documentation, and potentially in other related packages as well. That said, as long as we have some kind of subsection that is more specifically about different denoising approaches, I'm comfortable changing the file.
The rejected components are fit to the data without regard to the regressors of interest.
True, but the parameter estimates come from all components in the mixing matrix:
https://github.com/ME-ICA/tedana/blob/0f04f76bb51fc2ba8117e703c0ee0c9117c2388c/tedana/io.py#L372
Wouldn't that make it non-aggressive denoising?
Perhaps this can be reorganized to show how to load the ICA mixing matrix and component table into python/AFNI/FSL and then regress out those data in isolation or in combination with other noise regressors.
Great point. I can do that with the Python portion at least.
Why are we including FSL if FSL currently is currently so multi-echo unfriendly?
I just wanted to try it out with more than two tabs, but am happy to remove it since no one in the group uses it.
Good point on the betas
estimate. I think this makes me double-down on how aggressive vs non-aggressive can be confusing language. I'm also realizing we've sometimes used "aggressive"/"conservative" to describe variations in decision trees. We might need to be more precise (and unfortunately more wordy)
We can say that noise removal is regressing out:
- Rejected time series without regard to accepted time series (your aggressive)
- Rejected time series in a model that also fits accepted time series (your non-aggressive) (effectively splits the shared signals of accepted and rejected time series)
- Only the parts of rejected time series that aren't in the accepted time series (your orthogonalization) Note: rejected time series could be just what is rejected by tedana or also include things like motion or respiratory regressors.
If you want three tabs, would there be any difference between fmriprep and python?
I think it's important that we incorporate external nuisance regressors and regressors of interest, but I'm not sure if that should go in the main part of each section or in separate parts. Maybe separate tabs? Like:
[ Python ] [ Python with external regressors ] [ AFNI ] [AFNI with external regressors ]
WDYT? Would having separate sections, like Nonaggressive denoising, followed by Nonaggressive denoising with external regressors be better?
If you want three tabs, would there be any difference between fmriprep and python?
There probably will be some differences in filenames once we merge in some of the changes we've been discussing with the fMRIPrep team, but I don't think the core operations (e.g., what type of file each element is stored in) will differ. That said, I think folks will start copying and pasting our example code into their own scripts, so it might be best to have examples for each of the major naming conventions, even when the core code remains the same.
I like your tabs because they explain a bit more clearly what each includes. I'd lean into your tab suggestions more and use more mathy terminology rather than "aggressive" "non-aggressive". That would mean something like "Remove all noise-correlated fluctuations" "Only remove noise-correlated fluctuations that aren't correlated with fluctuations in accepted components" (That's probably a bit too wordy)
@handwerkerd what difference, if any, is there between orthogonalizing the rejected components w.r.t. the accepted ones and including both rejected and accepted components in the initial regression (i.e., nonaggressive denoising)?
I'd lean into your tab suggestions more and use more mathy terminology rather than "aggressive" "non-aggressive". That would mean something like "Remove all noise-correlated fluctuations" "Only remove noise-correlated fluctuations that aren't correlated with fluctuations in accepted components" (That's probably a bit too wordy)
I've added the longer headings and we can iterate on the descriptions. The biggest things I want to focus on are when and why someone would choose each approach, but I don't know enough about each to say.
@handwerkerd what difference, if any, is there between orthogonalizing the rejected components w.r.t. the accepted ones and including both rejected and accepted components in the initial regression (i.e., nonaggressive denoising)?
Maybe @CesarCaballeroGaudes and @smoia want to comment on this.
I hesitated to tag Cesar because I remember him very patiently explaining it to me in DC and I've completely forgotten it all! 😆
I hesitated to tag Cesar because I remember him very patiently explaining it to me in DC and I've completely forgotten it all! 😆
I believe he wanted to contribute to this PR.
This isn't quite right but hopefully it helps explain. Accepted components X includes fluctuations A, B, C Rejected components Y includes fluctuations C, D, E Aggressive denoising removes Y without regard to X, therefeore, C, D, & E are all removed. If A-E are all fit in the model, then, when you remove the rejected comopnents, you removed 0.5*C, D, & E since C is split. If you Y from X first, then you remove anything in Y that's common to X. In this case, only D & E are removed.
Hey, I'm listening 😄 (in fact I receive all TEDANA notifications). I was thinking for working on this PR or the AROMA during the forthcoming BHD
As @handwerkerd explains, the main difference between orthogonalizing and non-aggressive is that with orthogonalization, all the variance common across the accepted and rejected components is put in the accepted basket and therefore it is not removed in the nuisance regression. In contrast, with the non-agressive denoising, the shared variance is smartly split during the estimation process and therefore some amount is removed.
Looking at the RTD artifacts, Firefox and Safari ignore changes I made to theme_overrides.css
in this PR, but Chrome doesn't. It's weird, but not a big deal. Still, if anyone is willing to take a look at it, I would appreciate it.
In terms of the content, please feel free to make any suggestions you want to the code. I'll try to distinguish between the three approaches soon, but direct suggestions might be better.
@tsalo Are the overrides suppose to make a border around the topics look like this?
If so, I see it on Chrome and Firefox. Maybe some browsers are caching the older css---could try clearing the cache before reloading?
Yeah, it's supposed to go around topics, but I didn't realize that the TOC at the top of that file counted as a topic. I mostly just wanted it around the one I added to the denoising page (see below). If there are other elements that are technically "topics" then that's a whole new can of worms.
I guess I can just drop the new CSS and use an admonition with a custom title... 😞
The following change to the overrides should exclude the TOC:
- div.topic {
+ div.topic:not(.contents) {
I didn't spot the border anywhere else.
@notZaki thanks for the recommendation. I ended up playing with the general admonition settings instead of topic.
Just to check in: Is this now ready for review ?
I'm not sure what's going on with the linting CI step, but, other than that, I think this PR looks good.
I've decided to drop the AFNI and regular Python (without external regressors) tabs, since they seemed to be overcomplicating things.
Here's the latest version: https://tedana--823.org.readthedocs.build/en/823/denoising.html
I still wish our recommendations for which type of denoising to use were more explicit (e.g., in situation A, use denoising strategy 1), but I think this is pretty good now. I'd love everyone's input.
I don't think we can make much more specific recommendations. So much of this has to do with study specific goals. That is, for the specific goals of a study, is it worse to remove something that shouldn't be removed or keep something that should be removed.
Sorry for the extra commit! I accidentally used a markdown-style link instead of rst. I'll merge once CI passes though, so no need to re-review.