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

Support TFCE thresholding in CBMA algorithms #655

Closed
wants to merge 28 commits into from
Closed

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Mar 9, 2022

Closes #145.

Changes proposed in this pull request:

  • Add a tfce option to CBMAEstimator.correct_fwe_montecarlo that calculates TFCE-based FWE corrected maps.
  • Add a tfce option to MKDAChi2.correct_fwe_montecarlo as well.
  • Add a function, nimare.meta.utils.calculate_tfce, which calculates TFCE on a 3D numpy array.

@tsalo tsalo added enhancement New feature or request ibma Issues/PRs pertaining to image-based meta-analysis cbma Issues/PRs pertaining to coordinate-based meta-analysis labels Mar 9, 2022
@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #655 (cb7fcfc) into main (6cd751e) will increase coverage by 0.24%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #655      +/-   ##
==========================================
+ Coverage   85.28%   85.53%   +0.24%     
==========================================
  Files          40       40              
  Lines        4513     4589      +76     
==========================================
+ Hits         3849     3925      +76     
  Misses        664      664              
Impacted Files Coverage Δ
nimare/meta/cbma/ale.py 94.08% <ø> (ø)
nimare/meta/cbma/base.py 96.80% <100.00%> (+0.27%) ⬆️
nimare/meta/cbma/mkda.py 97.32% <100.00%> (+0.29%) ⬆️
nimare/meta/utils.py 57.38% <100.00%> (+4.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cd751e...cb7fcfc. Read the comment docs.

@tsalo tsalo removed the ibma Issues/PRs pertaining to image-based meta-analysis label Mar 10, 2022
@tsalo tsalo changed the title Support TFCE thresholding in CBMA and IBMA algorithms Support TFCE thresholding in CBMA algorithms Mar 10, 2022
@tsalo
Copy link
Member Author

tsalo commented Mar 10, 2022

Actually not going to touch IBMA any time soon.

nimare/meta/utils.py Outdated Show resolved Hide resolved
@tsalo tsalo marked this pull request as ready for review March 10, 2022 17:22
@tsalo
Copy link
Member Author

tsalo commented Mar 19, 2022

One question I have is whether the maximum statistic null distributions should be separated by sign for two-sided tests. For example, for the MKDAChi2, which can have positive and negative chi-squared values, should we retain maximum negative cluster size and maximum positive cluster size distributions separately?

I'm thinking about this primarily because of what I read in Albajes-Eizagirre et al. (2019):

SDM-PSI can currently save four different maximum statistics from the image of z-values: the largest z-value (i.e., voxel-based statistics), the maximum cluster size or mass after thresholding with a used-defined z- value (i.e., cluster-size or mass statistics) (Bullmore et al., 1999), and the maximum TFCE (Smith and Nichols, 2009). We have implemented TFCE in our software so that users who do not have FSL installed will still be able to use TFCE in SDM-PSI.

This procedure theoretically only tests hypothesis in one direction (e.g., patients > controls), but not the hypothesis in the other direction (e.g., patients < controls), and thus, the procedure should be conducted twice, one for each direction. However, given that the hypotheses are complementary and a permutation test is computationally consuming, SDM-PSI saves two numbers from each permutation: one for the positive hypothesis (e.g., the highest z-value) and one for the negative hypothesis (e.g., the lowest z-value), in two separate null distributions.

corr = FWECorrector(method="montecarlo", n_iters=5, n_cores=1)
corr = FWECorrector(method="montecarlo", n_iters=5, n_cores=1, tfce=True)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test now takes ~90 seconds locally. 😬

@jdkent
Copy link
Member

jdkent commented May 11, 2022

relevant publication from Simons group: https://pubmed.ncbi.nlm.nih.gov/35535616/

@tsalo
Copy link
Member Author

tsalo commented May 12, 2022

Thanks! I knew Simon was working on TFCE in ALE, but I didn't realize there was a paper out already. Based on the abstract, it may not be worth it to merge this PR, but on the other hand I do want NiMARE to be useable for that kind of study in the future...

@tsalo
Copy link
Member Author

tsalo commented May 12, 2022

I'm thinking that maybe I should close this PR, but also open a separate one that adds a note to the documentation about TFCE and points to this one. That way, if someone does want to use TFCE in NiMARE, they can refer to code that (at least at one point) worked, and they'll also see why we chose not to integrate it into the package.

I am also hopeful that we will be able to implement a much faster probabilistic TFCE in the future (like MNE), and that approach might not have the same issues noted in Simon's paper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cbma Issues/PRs pertaining to coordinate-based meta-analysis enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include TFCE thresholding
2 participants