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

Adds notebook for Jaccard Similarity #71

Merged

Conversation

rlratzel
Copy link
Contributor

This PR adds a notebook to demonstrate nx-cugraph's support for nx.jaccard_coefficients() by implementing a simple movie recommendation system based on MovieLens data.

This notebook will also be made available in Google Colab and referenced in an upcoming blog.

@rlratzel rlratzel added this to the 25.02 milestone Jan 30, 2025
@rlratzel rlratzel requested review from eriknw and nv-rliu January 30, 2025 10:26
@rlratzel rlratzel self-assigned this Jan 30, 2025
@rlratzel rlratzel requested a review from a team as a code owner January 30, 2025 10:26
@rlratzel rlratzel added doc Improvements or additions to documentation non-breaking Introduces a non-breaking change labels Jan 30, 2025
Copy link
Contributor

@eriknw eriknw left a comment

Choose a reason for hiding this comment

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

Looks good! Only a couple minor grammar/spelling nits.

Since timing is such a key point of the notebook, I think it would be nice to either include the outputs in the notebook or include some timing information in the discussion. I suspect most readers will be "fly by" skimmers and won't run the notebook themselves.

Also, can we provide a link to open this notebook in colab?

Copy link
Contributor

@nv-rliu nv-rliu left a comment

Choose a reason for hiding this comment

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

Just left some wording suggestions. Looks great!

notebooks/demo/nx_cugraph_jaccard_movie_recommender.ipynb Outdated Show resolved Hide resolved
notebooks/demo/nx_cugraph_jaccard_movie_recommender.ipynb Outdated Show resolved Hide resolved
notebooks/demo/nx_cugraph_jaccard_movie_recommender.ipynb Outdated Show resolved Hide resolved
notebooks/demo/nx_cugraph_jaccard_movie_recommender.ipynb Outdated Show resolved Hide resolved
notebooks/demo/nx_cugraph_jaccard_movie_recommender.ipynb Outdated Show resolved Hide resolved
notebooks/demo/nx_cugraph_jaccard_movie_recommender.ipynb Outdated Show resolved Hide resolved
Copy link
Contributor

@nv-rliu nv-rliu left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@eriknw eriknw left a comment

Choose a reason for hiding this comment

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

jaccard_coefficients (several places) should be changed to jaccard_coefficient.

@nv-rliu nv-rliu requested a review from eriknw February 4, 2025 08:09
@rlratzel
Copy link
Contributor Author

rlratzel commented Feb 4, 2025

/merge

@rapids-bot rapids-bot bot merged commit 114eb69 into rapidsai:branch-25.02 Feb 4, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Improvements or additions to documentation non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants