Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements smoothing and parallelisation #12

Merged
merged 14 commits into from
Sep 27, 2023
Merged

Implements smoothing and parallelisation #12

merged 14 commits into from
Sep 27, 2023

Conversation

jleechung
Copy link
Collaborator

@jleechung jleechung commented Jul 25, 2023

This PR implements label smoothing and parallel clustering.

Other changes include:

  • Filtering out samples with NAs in their features on init.
  • Seed setting for clustering
  • Version dependency on leidenAlg (>1.1.0) for compatibility with igraph
  • Neighborhood-sampling (see arguments sample_size, sample_renorm, and seed for function ComputeBanksy)
  • Replacing M with compute_agf and use_agf as arguments for specifying which harmonics to compute

This will be the last release / update for BANKSY with BanksyObject. Next release will adopt SingleCellExperiment as a container.

DESCRIPTION Show resolved Hide resolved
@@ -755,3 +768,8 @@ subsampler = function(knnDF,
if (sample_renorm) x[, weight := weight / sum(weight), by = from]
data.table(x)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For controlling harmonic usage, replace M with compute_agf (and use_agf).
This function converts the boolean flags compute_agf and use_agf to the corresponding M value.
M can still be used in a backward-compatible manner in ComputeBanksy, RunBanksyPCA, RunBanksyUMAP and ClusterBanksy; if it is provided, it overwrites compute_agf (or use_agf). This is documented as advanced usage.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@jleechung jleechung requested review from vipulsinghal02 and removed request for vipulsinghal02 September 4, 2023 06:46
}
if (is.list(bank@own.expr)) {
# Multiple datasets
return(bank)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose down the road we will also do smoothing for the multi dataset mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes!

Copy link
Collaborator

@vipulsinghal02 vipulsinghal02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. Let's proceed with the merge.

@jleechung jleechung merged commit 7ba2a8a into main Sep 27, 2023
4 checks passed
@jleechung jleechung deleted the dev branch September 27, 2023 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants