Conversation
Remove shape-only and attribute-only tests in favour of the correctness and invariant tests already present. Reuse the shared `sample_4d_volume` fixture where a single recording suffices.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new confusius.connectivity.CAP estimator implementing co-activation pattern (CAP) analysis via k-means clustering of fUSI volumes, including temporal dynamics metrics and cluster-count selection utilities.
Changes:
- Introduces
CAPclass with cosine/correlation (custom spherical k-means) and euclidean (sklearnKMeans) clustering backends. - Adds temporal metrics computation (fraction, counts, persistence, transitions) and
select_n_clusters()helper with multiple criteria. - Adds unit tests for
fit(),predict(), and temporal metrics; exportsCAPfromconfusius.connectivity.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/confusius/connectivity/cap.py | New CAP estimator, clustering routines, temporal metrics, and cluster selection helper. |
| src/confusius/connectivity/init.py | Exposes CAP in the connectivity public API. |
| tests/unit/test_connectivity/test_cap.py | Adds unit tests covering CAP fitting, prediction, and metrics. |
| docs/includes/abbreviations.md | Adds CAP/CAPs/SSQ abbreviations for documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Fix empty-cluster zero-vector bug: mask empty centers to -inf before argmax so they cannot beat valid centers with negative similarity - Store _spatial_dims during fit; transpose recordings in predict to guarantee feature alignment regardless of input dim order - Fix docstring: caps_.attrs["long_name"] is "CAP" not "Co-activation patterns" - Rename frames → volumes throughout (function, units string, docstrings, variable name, test assertions and comments)
…or tests Add tests for update_rule='mean', metric='cosine', metric='euclidean' predict, NaN input, invalid update_rule, and all select_n_clusters methods and error conditions.
Two changes: 1. fit() and select_n_clusters(): replace xr.concat() with np.concatenate() on stacked numpy arrays, then preprocess in-place. This eliminates one full-data copy for correlation/cosine metrics (~2 GB savings at typical scale of 50 recordings × 300 volumes × 32k voxels). 2. predict() euclidean path: replace (n_samples × n_caps × n_features) 3D broadcast with the identity ||x-c||² = ||x||² + ||c||² - 2x·c, reducing peak allocation from ~236 MB to ~7 KB per recording.
Add 11 tests targeting previously uncovered paths: - empty cluster warning (also exercises coincident-center init branch, empty-mask branch in Lloyd loop, and empty-cluster cleanup) - mean update rule in the cosine k-means loop - cosine predict path (_preprocess in_place=False) - NaN input in predict() - integer n_init (multiple restarts) and invalid n_init - select_n_clusters(): euclidean KMeans path, invalid metric/update_rule, empty recordings list, NaN input Remaining 3% is unreachable code (RuntimeError guard, dead _find_elbow single-value branch, external Rich Progress integration).
- _run_multi_cosine_kmeans: replace None-sentinel initialization with direct assignment from the first seed; remove the unreachable RuntimeError guard that required best_centers to be None after at least one k-means run. - _find_elbow: remove n==1 early-return; select_n_clusters validates len(cluster_range) >= 2 before calling this function.
Add test_best_restart_selected: with n_clusters=10 and random_state=0, restart 1 produces lower inertia than restart 0, so n_init=2 gives different labels than n_init=1. The test would fail if the best-restart update assignment were dropped. Data is constructed locally with a fixed seed to be independent of the session-scoped rng fixture's advancing state.
- Add `scores_` attribute (list of DataArrays parallel to `labels_`) computed by `fit()`: cosine similarity for correlation/cosine metrics, negative L2 distance for euclidean (higher = stronger assignment) - Add `score_samples()` method mirroring `predict()` for scoring new data - Add `score_threshold` parameter to `compute_temporal_metrics()`: volumes below threshold are censored (treated as -1) so they act as episode breaks and are excluded from all metric numerators while the total-volume denominator stays fixed, matching the motion-scrubbing convention - Change default `update_rule` from "weighted" to "mean", the theoretically correct update for spherical k-means; update docstring accordingly
Add Parameters, Returns, and Notes sections to _relative_luminance and _auto_fg_color; include WCAG 2.1 spec link in _relative_luminance. Expand _create_deterministic_time_series docstring with Returns section.
This reverts commit e5c19f9.
…r_confounds A Dask array chunked along time produced silently wrong (all near-zero) variance values, causing an unhelpful "all voxels have zero variance" error instead of a clear indication of the root cause. validate_time_series already checks for time chunking, so passing the signal through it is sufficient.
…unds noise_mask.values.flatten() used the mask's own dimension order, which silently misaligned with signals.stack(space=spatial_dims) when the mask had its spatial dims in a different order. validate_mask catches name and coordinate mismatches but not ordering differences, so wrong (background, zero-variance) voxels were selected. Fixed by stacking the mask with the same spatial_dims as the signals before extracting values.
sdiebolt
force-pushed
the
main
branch
7 times, most recently
from
May 14, 2026 03:11
601802d to
c92fe20
Compare
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 4 to 8. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v4...v8) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '8' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Samuel Le Meur-Diebolt <samuel@diebolt.io>
…158) Bumps [marocchino/sticky-pull-request-comment](https://github.com/marocchino/sticky-pull-request-comment) from 2 to 3. - [Release notes](https://github.com/marocchino/sticky-pull-request-comment/releases) - [Commits](marocchino/sticky-pull-request-comment@v2...v3) --- updated-dependencies: - dependency-name: marocchino/sticky-pull-request-comment dependency-version: '3' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Samuel Le Meur-Diebolt <samuel@diebolt.io>
* test(xarray): add wrapper forwarding tests * test(xarray): cover remaining accessor branches * test(xarray): split connectivity wrapper forwarding cases
Co-authored-by: sdiebolt <3774850+sdiebolt@users.noreply.github.com>
* feat(datasets): add Huang 2025 template fetcher * docs(changelog): add PR link for Huang template * docs(citing): harmonize citation style and licenses * cite peer-reviewed verion for Cybis Pereira 2026 --------- Co-authored-by: Felipe Cybis Pereira <felipe.cybispereira@gmail.com>
* feat(validation): add generic dataarray validators Introduce validate_fusi_dataarray for shared structural validation, refactor IQ validation to build on it, and rename validate_iq to validate_iq_dataarray. * test(iq): fix stale error message patterns and non-monotonic time fixtures Update three match strings to reflect new validate_fusi_dataarray messages, and fix two time coordinate arrays that were accidentally non-monotonic (duplicate/decreasing values), causing the general validator to fire before the Butterworth-specific regularity check. * fix(validation): import spacing helper from coordinates * refactor(validation): reuse fusi validator in registration * refactor(registration): enforce strict fusi validation * test(validation): clarify coord policy and enforce invariants * refactor(validation): require numeric core coordinates * docs(changelog): note validation API breaking changes * refactor(validation): scope regular-spacing checks Add regular_spacing_dims selector to validate_fusi_dataarray with spatial default. Update motion to enforce spatial-only regular spacing. Integrate validation in smooth_volume while preserving missing-coordinate behavior. Standardize shared test sample DataArrays with spatial voxdim metadata and migrate validation tests to shared 3D+t fixture with explicit 2D+t coverage. * refactor(validation): refine regular-spacing dim modes Support regular_spacing_dims values spatial/core/all/explicit sequence. Spatial remains default. Core checks canonical dims; all and explicit dims skip non-numeric coordinates. Update validation tests to use shared 3D+t fixture patterns and cover new mode semantics. * test(spatial): cover smooth validation re-raise path Add regression test for smooth_volume branch that re-raises non-missing-coordinate validation errors from validate_fusi_dataarray. * feat(validation): simplify regular-spacing dim selection * test(xarray): use 3dt fixtures in wrapper tests * refactor: put dimension coordinate checks inside `_validate_dimension_coordinate` * Explicify missing coordinates for dims when required spacing * test(validation): cover single-dim regular spacing selector --------- Co-authored-by: Felipe Cybis Pereira <felipe.cybispereira@gmail.com>
…el GLM (#155) * feat(connectivity): optimize masked SeedBasedMaps path * test(decomposition): cover masked reconstruction paths * refactor(decomposition): use unmask for masked reconstruction * test(decomposition): tolerate float drift in PCA noise variance * test(connectivity): cover masked SeedBasedMaps branches * refactor(glm): unify masked first-level handling * fix(masking): align masked dimension ordering * refactor(validation): add exact mask dim option Add to and use it in GLM and decomposition to remove duplicated full-spatial-dim checks. * fix(tests): fix assertion matching * refactor(decomposition): apply mask via extract_with_mask Replace manual stack/reindex/ravel masking in _prepare_data with the extract_with_mask helper, so the unmask round-trip uses its matching counterpart. require_exact_dims on validate_mask already guarantees mask/data dim order, making the explicit coordinate alignment redundant. Drop the X_stacked plumbing through _store_fit_metadata and remove the write-only _feature_coord_, _full_feature_coord_, and _feature_mask_ attributes; reconstruction relies solely on _reconstruction_mask_. * docs(changelog): add mask argument entry for #155 --------- Co-authored-by: Felipe Cybis Pereira <felipe.cybispereira@gmail.com>
* chore: prepare v0.3.0 release * chore: misc formatting
Bumps [actions/github-script](https://github.com/actions/github-script) from 8 to 9. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](actions/github-script@v8...v9) --- updated-dependencies: - dependency-name: actions/github-script dependency-version: '9' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: sdiebolt <3774850+sdiebolt@users.noreply.github.com>
# Conflicts: # docs/includes/abbreviations.md # src/confusius/signal/confounds.py
References must come before Examples per numpydoc GL07; CI lint was failing.
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
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 #114.
Summary
CAPclass implementing co-activation pattern analysis via k-means clustering of fUSI volumes"correlation"(Pearson, default),"cosine", and"euclidean"KMeansfor Euclideanfit()accepts a list of recordings (or a singleDataArray); labels are stored per-recording so recording boundaries are respected for temporal metricspredict()assigns new recordings to fitted CAPscompute_temporal_metrics()returns temporal fraction, episode counts, persistence (time-coord-aware, handles irregular sampling), transition frequency, and transition probability matrixselect_n_clusters()helper with elbow, silhouette, Davies-Bouldin, and variance-ratio criteria_prepare_datawith a clear error pointing to background-voxel z-scoring as the likely cause