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

DBSCAN Clustering Algorithm #1036

Merged
merged 2 commits into from
Feb 23, 2022
Merged

DBSCAN Clustering Algorithm #1036

merged 2 commits into from
Feb 23, 2022

Conversation

asvsfs
Copy link
Contributor

@asvsfs asvsfs commented Aug 6, 2020

Dear ml5 community,

I'm making a Pull Request(PR). Please see the details below.

A good PR 🌟

β†’ Step 1: Which branch are you submitting to? 🌲

Development (for new features or updates), Release (for bug fixes), or ___________?

Development

β†’ Step 2: Describe your Pull Request πŸ“

Fixing a Bug? Adding an Update? Submitting a New Feature? Does it introduce a breaking change?

I've added a simple DBSCAN clustering algorithm to the models , it is influenced by jDBSCAN and WIKIPEDIA definition of the algorithm

A great PR 🌟🌟

β†’ Step 3: Share a Relevant Example πŸ¦„

Here's some example code or a demonstration of my feature as a part of this pull request, a separate pull request, in the
https://editor.p5js.org, or codepen/jsfiddle/etc...

D3 Example is included in the project.

The best PR 🌟🌟🌟

β†’ Step 4: Screenshots or Relevant Documentation πŸ–Ό

Here's some helpful screenshots and/or documentation of the new feature

I've tried to Tensorized it as much as i could since i'm completely new to Tensorflow , any help is welcome to improve the code πŸ‘
image

@asvsfs asvsfs marked this pull request as ready for review August 6, 2020 18:24
@asvsfs
Copy link
Contributor Author

asvsfs commented Aug 6, 2020

I'm not sure why but some test unrelated to this PR don't make it (ObjectDetector..)

@joeyklee
Copy link
Contributor

@asvsfs - This is amazing! Wow super cool. This has been on my list for a while now and am so glad you picked this up.

I won't have time to review for awhile in depth, but I will let you know when I am able to test and give feedback. On a quick first glance, things are looking good! I'm excited to have a look.

Thanks for your contribution!

};
this.lastClusterId = 0;
this.status = [];
this.load(dataset).then(callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi! I'm wondering why there are two calls to this.load() on lines 67 and 68. Is it possible that you only wanted to call this once?
If so, I think line 67 does not need to be included. We can just keep line 68.

Copy link
Contributor Author

@asvsfs asvsfs Aug 28, 2020

Choose a reason for hiding this comment

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

@joeyklee Corrected.

// Step 4:
// use the fancy d3 to make magic
function makeChart() {
const dataset = dbscanModel.dataset;
Copy link
Contributor

Choose a reason for hiding this comment

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

Small suggestion: we can use object destructuring here:

const { dataset } = dbscanModel;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

@bomanimc bomanimc changed the base branch from development to main November 9, 2020 22:39
Copy link
Contributor

@joeyklee joeyklee left a comment

Choose a reason for hiding this comment

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

Dear @asvsfs - thanks so much for this feature addition and for your patience and understanding. This is such an exciting addition to ml5.

@joeyklee
Copy link
Contributor

NOTE: I will open an issue for a bug I noticed with how our examples.json gets created. I found that the dbscan examples weren't showing for some reason and it is because the scripts/update-examples-json.js is looking for a directory in the examples/p5 directory to key off of. I will merge this in w/ a quick fix using .gitkeep in examples/p5/DBSCAN

@joeyklee joeyklee merged commit 6a01966 into ml5js:main Feb 23, 2022
@joeyklee joeyklee added the SEMVER/minor a version tag for a minor release change label Feb 23, 2022
@joeyklee
Copy link
Contributor

@all-contributors please add @asvsfs for code example

@allcontributors
Copy link
Contributor

@joeyklee

I've put up a pull request to add @asvsfs! πŸŽ‰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request SEMVER/minor a version tag for a minor release change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants