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

[Algorithms] Add clustering algorithms #20

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

tryuan99
Copy link
Collaborator

@tryuan99 tryuan99 commented Jan 3, 2025

This PR adds some clustering algorithms and helper classes, e.g., the Cluster class and the IClusterer interface. Constrained clustering algorithms are also included and constrain the size and radius of each cluster.

  • $k$-means clustering: Standard $k$-means clustering given the number of desired clusters $k$
  • Constrained $k$-means clustering: $k$-means clustering with size and radius constraints
  • Agglomerative clustering: standard agglomerative clustering with the size and radius constraints as stopping conditions

@tryuan99 tryuan99 requested a review from daniellovell January 5, 2025 05:20
tryuan99 and others added 4 commits January 4, 2025 22:36
In my initial review I couldn't understand how the cluster membership was being cleared, so I wrote this test.
Perhaps its useful to retain, despite the fact that your implementation works fine and the test passes with no modification to the algo.
Copy link
Member

@daniellovell daniellovell left a comment

Choose a reason for hiding this comment

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

In my initial review I couldn't understand how the cluster membership was being cleared, so I wrote a new KMeansClustererTest.
Perhaps its useful to retain, despite the fact that your implementation works fine and the test passes with no modification to the algo.


[Test]
public void TestCentroid() {
const float radius = 3;
Copy link
Member

Choose a reason for hiding this comment

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

Unused variable prompting linter warning


[Test]
public void TestRecenter() {
const float radius = 3;
Copy link
Member

Choose a reason for hiding this comment

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

Unused variable prompting linter warning

@daniellovell
Copy link
Member

I recommend updating the branch from master as part of the PR

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