fix: select highest-variance features in pgx.FindClusters - #513
Merged
Conversation
- order(rowSds(X)) sorted ascending, so top.sd selected the LOWEST variance features; on CPTAC_GBM this picked genes with SD 0.004-0.118 against a dataset max of 3.83, making clusters indistinguishable from noise - use PC scores (v * d) instead of the unit-norm right singular vectors, which weighted a 46% variance component the same as pure noise and cancelled out any benefit from correct feature selection - add nstart = 20 to kmeans so runs are reproducible across seeds; adjusted Rand index against known sample labels goes from ~0 to 0.92-1.00 and reseed stability from 0.01-0.20 to 1.00 on five local datasets
zitoa
approved these changes
Aug 31, 2026
Member
|
@phisanti Approved — CI is green and the branch merges cleanly into `edgy`, so this is ready to merge. 🚀 (Heads-up: the three `.Rd` files in this PR are just roxygen doc-regeneration and already match what is on `edgy` — no behavioral change, so nothing to worry about there.) |
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
3 participants
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.
While clustering the sepsis multicohort dataset (Tsalik EL, Langley RJ, Dinwiddie DL, et al. An integrated transcriptome and expressed variant analysis of sepsis survival and death. Genome Med. 2014;6(11):111. doi:10.1186/s13073-014-0111-5) I got three "endotypes" that split by cohort instead of by biology, and reran the same call with a different seed and got a different answer. That sent me to the backend.
pgx.FindClusters()had three problems stacked on top of each other:R/pgx-cluster.R:275sorted by SD ascending, sotop.sdpicked the lowest-variance features. On CPTAC_GBM (a dataset from axel) it selected genes with SD 0.004-0.118 against a dataset max of 3.83.:285usedout$vfrom irlba directly. Those are unit-norm, so a component holding 46% of the variance got the same weight as pure noise. This one cancelled out any benefit from fixing Packages that are required but cant be installed #1.:296calledkmeans()with nonstart, so a single random init decided the result.Fix is 3 lines. But I'd like a second opinion before merging:
pgx.FindClusters()has no callers in playbase; it's used in omicsplayground for the timeseries gene modules (board.timeseries/R/board_server.R:78).nstart = 20makes kmeans ~20x slower. Fine at these sizes, but say so if it's called on something much bigger.npca <- min(npca, dim(X) - 1)at:281alone. It lets npca reachn_samples - 1, which makes irlba warn about computing too many singular
values. Harmless once fix issue #232 on OPG #2 is in, but flagging it.
Two related things I hit and did not touch here, happy to open separate issues:
find.clustersis hardcodedFALSEat:216sopgx$samples$clusteris never populated by the current pipeline.