[ENH] ale subtraction scale improvements - #1002
Merged
jdkent merged 4 commits intoApr 3, 2026
Merged
Conversation
Use empirical voxelwise SCALE nulls and add voxel-level Monte Carlo FWE support. Stream ALESubtraction voxelwise null updates to reduce storage overhead, speed shared cluster and null-distribution helpers, and deduplicate shared permutation summarystat generation in the CBMA base path.
Update the MACM and CBMA example snippets to show voxel-level Monte Carlo FWE correction for SCALE and clarify that cluster-level correction is not supported.
Contributor
Reviewer's GuideRefactors ALESubtraction and SCALE CBMA estimators to use streamed permutation tail counts and optimized permutation paths, adds voxel-wise Monte Carlo FWE correction for SCALE, and aligns core statistical helpers and tests with the new behavior while simplifying some histogram-based APIs and cluster-mass utilities. Class diagram for updated ALESubtraction permutation logicclassDiagram
direction TB
class CBMAEstimator {
+masker
+kernel_transformer
+null_distributions_
+_compute_summarystat(data)
+_compute_permutation_summarystat(iter_ijk, iter_df)
+_compute_null_montecarlo_permutation(iter_ijk, iter_df, bin_edges)
+_correct_fwe_montecarlo_permutation(iter_ijk, iter_df, vfwe_only)
}
class PairwiseCBMAEstimator {
+masker
+kernel_transformer
+null_distributions_
}
CBMAEstimator <|-- PairwiseCBMAEstimator
class ALESubtraction {
+n_iters
+n_cores
+vfwe_only
+memmap_filenames
+null_distributions_
+inputs_
+_collect_ma_maps(coords_key, maps_key, return_type)
+_compute_summarystat_est(ma_values)
+_combine_ma_maps(ma_maps1, ma_maps2)
+_run_permutation(i_iter, n_grp1, ma_arr)
+_fit(dataset1, dataset2)
+correct_fwe_montecarlo(result, voxel_thresh, n_iters, n_cores, vfwe_only)
}
PairwiseCBMAEstimator <|-- ALESubtraction
class _finalize_alediff_tail_counts {
+_finalize_alediff_tail_counts(left_counts, right_counts, n_iters)
}
class _collect_ale_masked_ma_maps {
+_collect_ale_masked_ma_maps(estimator, coords_key, maps_key)
}
ALESubtraction --> _collect_ale_masked_ma_maps : uses
ALESubtraction --> _finalize_alediff_tail_counts : uses
ALESubtraction --> CBMAEstimator : uses base methods
class ALEKernel {
+transform(data, masker, return_type)
}
ALESubtraction --> ALEKernel : uses
class collect_csr_ma_maps {
+collect_csr_ma_maps(estimator, coords_key, maps_key)
}
_collect_ale_masked_ma_maps --> collect_csr_ma_maps : wraps
class _csr_row_max {
+_csr_row_max(ma_values)
}
_collect_ale_masked_ma_maps --> _csr_row_max : caches_max
class _calculate_cluster_measures {
+_calculate_cluster_measures(arr3d, threshold, conn, tail)
}
ALESubtraction --> _calculate_cluster_measures : for_cluster_FWE
class null_to_p {
+null_to_p(test_values, null_distribution, tail, symmetric)
}
class p_to_z {
+p_to_z(p_values, tail)
}
ALESubtraction --> null_to_p : for_FWE
ALESubtraction --> p_to_z : for_Zmaps
class DEFAULT_FLOAT_DTYPE {
}
ALESubtraction --> DEFAULT_FLOAT_DTYPE : dtype
Class diagram for updated SCALE estimator and Monte Carlo FWEclassDiagram
direction TB
class CBMAEstimator {
+masker
+kernel_transformer
+null_distributions_
+inputs_
+_compute_summarystat(data)
+_compute_permutation_summarystat(iter_ijk, iter_df)
+_compute_null_montecarlo_permutation(iter_ijk, iter_df, bin_edges)
+_correct_fwe_montecarlo_permutation(iter_ijk, iter_df, vfwe_only)
}
class SCALE {
+xyz
+n_iters
+n_cores
+null_distributions_
+inputs_
+_fit(dataset)
+_compute_summarystat_est(data)
+_prepare_permutations(n_iters)
+_prepare_permutation_args(coordinates)
+_scale_to_p(stat_values, scale_values)
+_run_permutation(iter_idx, voxel_ijk, iter_df, permutation_args)
+_iterate_permuted_stats(rand_idx, voxel_ijk, iter_df, permutation_args, n_cores)
+correct_fwe_montecarlo(result, voxel_thresh, n_iters, n_cores, vfwe_only)
}
CBMAEstimator <|-- SCALE
class ALEKernel {
+sample_size
+fwhm
+transform(data, masker, return_type)
}
SCALE --> ALEKernel : optional_kernel_transformer
class compute_ale_ma {
+compute_ale_ma(mask_img, voxel_ijk, kernel, exp_idx, sample_sizes, use_dict)
}
SCALE --> compute_ale_ma : uses_for_permutations
class get_ale_kernel {
+get_ale_kernel(mask_img, fwhm)
}
SCALE --> get_ale_kernel : prepares_kernel
class mm2vox {
+mm2vox(xyz, affine)
}
SCALE --> mm2vox : xyz_to_voxel_indices
class p_to_z {
+p_to_z(p_values, tail)
}
SCALE --> p_to_z : voxelwise_Z
class null_to_p {
+null_to_p(test_values, null_distribution, tail, symmetric)
}
SCALE --> null_to_p : in_correct_fwe_montecarlo
class DEFAULT_FLOAT_DTYPE {
}
SCALE --> DEFAULT_FLOAT_DTYPE : dtype
class FWECorrector {
+method
+n_iters
+n_cores
+transform(result)
}
FWECorrector --> SCALE : calls_correct_fwe_montecarlo
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="nimare/meta/cbma/ale.py" line_range="1272-1277" />
<code_context>
require_masked_csr(ma_values) if sp_sparse.isspmatrix(ma_values) else ma_values
)
+ def _prepare_permutations(self, n_iters):
+ """Prepare shared SCALE permutation inputs."""
+ iter_df = self.inputs_["coordinates"].copy()
+ voxel_ijk = mm2vox(self.xyz, self.masker.mask_img.affine).astype(np.int32, copy=False)
+ permutation_args = self._prepare_permutation_args(iter_df)
+ rand_idx = np.random.choice(voxel_ijk.shape[0], size=(iter_df.shape[0], n_iters))
+ return iter_df, voxel_ijk, permutation_args, rand_idx
+
</code_context>
<issue_to_address>
**suggestion:** Clarify and/or rename `rand_idx` and `iter_idx` to avoid confusion between voxel indices and iteration indices.
In `_prepare_permutations`, `rand_idx` is shaped `(n_rows, n_iters)` and stores indices into `voxel_ijk`. Later, `_iterate_permuted_stats` passes `rand_idx[:, i_iter]` as `iter_idx` to `_run_permutation`, whose docstring calls it a "permutation index". Since these are voxel index arrays rather than scalar iteration indices, the current naming/docstring is misleading. Consider renaming (e.g., `voxel_idx_for_iter`) or updating the docstring to clearly indicate they are voxel indices.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1002 +/- ##
==========================================
- Coverage 85.53% 85.38% -0.15%
==========================================
Files 52 52
Lines 9255 9256 +1
==========================================
- Hits 7916 7903 -13
- Misses 1339 1353 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes # .
Changes proposed in this pull request: