feat: automatic K selection (CellCharter's ClusterAutoK) - #1276
feat: automatic K selection (CellCharter's ClusterAutoK) #1276selmanozleyen wants to merge 10 commits into
Conversation
…ce' for better clarity
`random_state` is dropped outright rather than deprecated: it is simply gone from the signatures, so passing it raises Python's own TypeError. The released defaults are preserved -- `seed` still defaults to 42, so calls that do not pass it stay reproducible exactly as before. Passing `seed=None` opts out. Internally `seed` now feeds `numpy.random.Generator`s: `spawn_generators` derives an independent generator per library and per resolution, and `rng_to_random_state` converts at the boundary of third-party APIs that take an int but not a Generator (scikit-learn, spatialleiden). Adding a library or a resolution therefore no longer shifts the others. Derived from 777449e on feat/cluster-auto-k.
Completes the rename, so `random_state` no longer appears as a squidpy parameter name anywhere. `WekaParams.random_state`, `VahadaneParams.random_state` and `_refine_with_background_classifier`'s parameter become `seed`, keeping their existing default of 0. No conversion helper is needed: these are plain `int | None` and go straight into scikit-learn, which accepts that. The `random_state=` keywords that remain are scikit-learn's own, on `RandomForestClassifier` and `NMF`.
selmanozleyen
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1276 +/- ##
==========================================
+ Coverage 78.60% 79.15% +0.55%
==========================================
Files 63 65 +2
Lines 9348 9562 +214
Branches 1559 1589 +30
==========================================
+ Hits 7348 7569 +221
+ Misses 1426 1419 -7
Partials 574 574
🚀 New features to boost your workflow:
|
|
I think a Bayesian GMM could also be very interesting for the same purpose: https://scikit-learn.org/stable/modules/generated/sklearn.mixture.BayesianGaussianMixture.html
The downside is that it's quite slow - not sure how hard it would be to make a fast GPU implementation for that @Intron7. |
| stable K is the one whose partition is least sensitive to adding a cluster. | ||
|
|
||
| Reimplements the K selection of CellCharter's ``ClusterAutoK``, using | ||
| :class:`~sklearn.mixture.GaussianMixture` for the fits. |
There was a problem hiding this comment.
Does this have to be tied to the GaussianMixture? We deliberately split the niche calling procedure such that arbitrary embeddings and clustering algos can be combined... In principle, why not use the same approach for Leiden?
There was a problem hiding this comment.
I didn't want to define a clusterer protocol publicly. And to have a hard-coded list of clusterers I wanted some opinions first. Bayesian GMM is a good hint
| previous_curve: np.ndarray | None = None | ||
| converged = False | ||
|
|
||
| for run in range(max_runs): |
There was a problem hiding this comment.
this is embarrassingly parallel. Probably the clustering algorithm has some sort of threading, too, but did you evaluate how well it scales to many cores? Maybe parallelizing at this level could make sense.
There was a problem hiding this comment.
I think we rely on the clustering algorithm to be utilizing the cores as much as possible so that any other threading would oversubscribe only. Also we can only parallelize runs if tolerance is 0, ie we don't early stop.
But from my small benches I didn't see much improvement than 1.5x tbh. And BLAS was already enough.
There was a problem hiding this comment.
I was planning to open a separate issue the next days to discuss a public "extensibility" interface for the niche algos. We can discuss it again in that context.
| # keyed by (run, K) rather than drawn in sequence, so appending a candidate K | ||
| # does not re-seed every other K's fits | ||
| random_state = int(np.random.SeedSequence([seed, run, k]).generate_state(1)[0]) | ||
| labels, nll = _fit_once(X, k, random_state, model_params) |
There was a problem hiding this comment.
The runs only differ at the level of random seeds. Would it make sense to combine this with e.g. bootstrapping?
2 participants
fixes: #1244
will add more description but I want to start somewhere @sarajimenez, @FrancescaDr these are all the things I could find in https://github.com/Jieran-S/hitchhikers_guide_to_spatial_project/blob/spatial_clustering/notebooks/Spatially_aware_clustering/sac.ipynb.
Anything else needed? If everything is there, how does this state look like for you?