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

Sparse KDE #221

Closed
Closed

Conversation

GardevoirX
Copy link
Contributor

@GardevoirX GardevoirX commented Feb 14, 2024

This PR introduces SparseKDE:

  • The class SparseKDE is located at src/skmatter/utils/_sparsekde.py. It mitigates the high cost of doing KDE for large datasets by doing KDE for selected data points (e.g. grid points sampled by farthest point-sampling). This class takes the original dataset as a parameter and fits the model using the sampled grid points.
  • There are two auxiliary classes and some auxiliary functions of SparseKDE stored in src/skmatter/utils/_sparsekde.py.
  • Two distance metrics compatible with PBC, pairwise_euclidean_distances and pairwise_mahalanobis_distances, are realized and stored in src/skmatter/metrics/pairwise.py.
  • Tests for SparseKDE and some auxiliary functions are stored in tests/test_neighbors.py. Tests for distance metrics are stored in tests/test_metrics.py.

I am not sure if the current API of SparseKDE is OK and if the auxiliary classes should be integrated into SparseKDE. Also, SparseKDE seems to be too large and complex. Perhaps it needs to be decomposed into smaller parts, but I have not figured out how.

Contributor (creator of PR) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

For Reviewer

  • CHANGELOG updated if important change?

📚 Documentation preview 📚: https://scikit-matter--221.org.readthedocs.build/en/221/

@PicoCentauri
Copy link
Collaborator

Thanks @GardevoirX for this work. I will take a look asap.

@PicoCentauri PicoCentauri deleted the branch scikit-learn-contrib:sparse-kde February 16, 2024 08:47
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