search

epurdom / clusterExperiment

R package of techniques for comparing clusterings of single-cell sequencing data
36 stars 13 forks source link

use fastcluster for hierarchical clustering #237

Open epurdom opened 6 years ago

epurdom commented 6 years ago

http://danifold.net/fastcluster.html

Should probably be as simple as calling the hclust from this package rather than stats.

drisso commented 6 years ago

Just implemented this in the feature/fastcluster branch.

It was indeed as easy as changing a few lines to use fastcluster::hclust.

Assuming it passes all checks, can I merge into develop?

drisso commented 6 years ago

And obviously it failed... :|

drisso commented 6 years ago

So apparently fastcluster::hclust() and stats::hclust() do not return the same exact value, because when running the vignette with the same parameters, with stats we get two clusters and with fast cluster only one. This causes an error in getBestFeatures which makes the build fail.

drisso commented 6 years ago

Perhaps we should wait on this since hclust() is not the main bottleneck.

epurdom commented 6 years ago

I agree we should wait. Perhaps they just have different defaults that we need to align…

On Apr 13, 2018, at 10:10 PM, Davide Risso notifications@github.com wrote:

Perhaps we should wait on this since hclust() is not the main bottleneck.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/epurdom/clusterExperiment/issues/237#issuecomment-381248143, or mute the thread https://github.com/notifications/unsubscribe-auth/AHXGVfWqWCoUvv3l24PeLUrZK1L1OXEYks5toQYagaJpZM4SsIDW.