-
Notifications
You must be signed in to change notification settings - Fork 313
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
fix: Compare Cluster and ClusterFast scores and speedup #892
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great @isaac-chung! A few surprising results here.
I might be afraid that the new method (resampling many documents) produces results closer to the original (which is nice), but what we are really interested in is that they produce the same model ordering and with the same (or better) power to differentiate between similar models.
Though resampling many times does seem to be quite cheap.
...ngual-e5-small/e4ce9877abf3edfe10b0d82785e83bdcb973e22e/TwentyNewsgroupsClustering.v3-2.json
Outdated
Show resolved
Hide resolved
...float__multilingual-e5-base/d13f1b27baf31030b7fd040960d60d909913633f/ArxivClusteringP2P.json
Show resolved
Hide resolved
…ts; add test significance'
Looking ahead to what's needed to close/merge this PR, the following should be completed first:
Anything else I'm missing? |
I think that it good |
5d55639
to
241f6c8
Compare
Added plotting scripts and plots here. Will remove them from this PR, then we should be good to merge. |
@KennethEnevoldsen just wanted a sanity ✅ before we merge, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments otherwise looks good - once these are in feel free to merge. Thanks for taking the time on this one!
…ion in docstrings
1) What
Part of #835 .
2) Hardware
These were run on a single A10.
3) Scores on ClusteringFast + Speedup
These are the Spearman's correlation scores and speedups from Clustering to ClusteringFast. The old notes are hidden within each expand.
4% n_samples for every task except for RedditClustering and StackExchangeClustering with 32768)
In addition, p-values for each model is reported.
Old notes comparing across tasks.
a) Current Implementation
b) New Commit: b8a5be4
c) New Commit: 6295dcc (swap max_documents_per_cluster and max_documents_to_embed values)
d) New Commit: 3a0d9c4 (c + increasing max_documents_per_cluster to 65536)
Old notes comparing across models.
a) Current Implementation
b) New Commit: b8a5be4
c) New Commit: 6295dcc (swap max_documents_per_cluster and max_documents_to_embed values)
d) New Commit: 3a0d9c4 (c + increasing max_documents_per_cluster to 65536)
Checklist
make test
.make lint
.