Add sum_across_studies to kda
#859
Conversation
|
oh shoot, do we actually not need the individual matrices for fwe correction? Looking over the code it seems like we don't, and the first thing done for mkdachi2 is summing across all the studies. |
|
Allright guys @JulioAPeraza @jdkent @tsalo, this is realy exciting. For MKDAChi2 we don't actually need study level modeled activiation. This realization means we can save massive amounts of memory and substantial time (2-3x speed up over an already optimized PR I was working on). When looping over studies, instead of saving all study level (M)KDA modeled activation (MA) maps, we can sum them in-place into a single MNI sized dense volume. This means no need to run I initally enable this for all MKDAKernels, but it seems to result in incorrect statisics for |
|
On a measly Macbook Air with 8GB of RAM, I was able to run
Someone check my logic in case I'm missing something! |
Co-authored-by: James Kent <[email protected]>
|
I was able to simplify the code further by just returning a dense matrix. Saves ~2 more seconds. I'm open to other stylistic suggestions though. |
nimare/meta/utils.py
Outdated
| all_values += study_values | ||
|
|
||
| # Set voxel outside the mask to zero. | ||
| all_values[~mask_data] = 0 |
There was a problem hiding this comment.
and this step is functionally replacing this
sphere_idx_inside_mask = np.where(mask_data[tuple(all_spheres.T)])[0]
all_spheres = all_spheres[sphere_idx_inside_mask, :]
Co-authored-by: James Kent <[email protected]>
|
|
||
| del ma_maps1 | ||
| n_selected = self.dataset1.coordinates["id"].unique().shape[0] |
There was a problem hiding this comment.
probably should have _collect_ma_maps return what the n_selected value is, all the coordinates for a particular experiment could exist outside the mask and the experiment would not be included.
|
Your changes look good to me, James. |
|
fantastic work! merging it in. |
* Simplify stacking * Fix typo * Remove vstack * Fix stacking * Remove @Profile * Use jit for _convolve_sphere * switch arrays to int32 where possible * reduce memory consumption * fix style * Simplify numba * Run black * �Mask outside space in numba * Add indicator later * Set value to input * Add `sum_across_studies` to kda (#859) * Resolve merge * Add sum aross studies * Remove @Profile * Only enable sum across studies for MKDA Chi Squared * Run black * Return dense for MKDACHiSquared * Update nimare/meta/utils.py Co-authored-by: James Kent <[email protected]> * Run black * Update nimare/meta/utils.py Co-authored-by: James Kent <[email protected]> * Format suggestion * change how number of studies and active voxels are found * add explicit dtype when creating image * make the comment clearer * add the kernel argument to the dictionary * bump minimum required versions * alternative way to sum across studies in a general way * fix arguments and style * pin minimum seaborn version --------- Co-authored-by: Alejandro de la Vega <[email protected]> Co-authored-by: James Kent <[email protected]> * manage minimum dependencies * Index within numba * Only allow sum_overlap if not sum_across_studies * Add unique index back * Remove @Profile * run black * ensure the methods for creating the kernel are equivalent --------- Co-authored-by: James Kent <[email protected]> Co-authored-by: Alejandro de la Vega <[email protected]>
sum_across_studiesreturns an array where study counts are sequentially added to a single dense matrix, rather than compiling an exhaustive matrix containg all study-level count data.This is much more effecient memory and compute wise, enabling MKDA Chi squared with Neurosynth as reference, using <1GB of memory.