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

feat: add usearch #2608

Merged
merged 10 commits into from
Sep 13, 2024
Merged

feat: add usearch #2608

merged 10 commits into from
Sep 13, 2024

Conversation

iammytoo
Copy link
Contributor

@iammytoo iammytoo commented Sep 11, 2024

  • Implement interface for additional ann algorithm “usearch”.

Description

  • Implemented internal/core/algorithm/usearch/*.go.
  • Edited Makefile and hack/docker/gen/main.go.

Related Issue

Versions

  • Vald Version: v1.7.13
  • Go Version: v1.23.0
  • Rust Version: v1.80.0
  • Docker Version: v27.1.1
  • Kubernetes Version: v1.30.3
  • Helm Version: v3.15.3
  • NGT Version: v2.2.4
  • Faiss Version: v1.8.0

Checklist

Special notes for your reviewer

Summary by CodeRabbit

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced version management for the usearch library, allowing users to check the current version.
    • Added installation support for usearch on both Linux and macOS systems.
  • Enhancements

    • Integrated usearch installation into the Docker build process.
    • Added a comprehensive Go API for managing vector indices with the usearch library.
    • Provided flexible configuration options for the usearch algorithm, enhancing usability.
  • Bug Fixes

    • Improved error handling with a custom error type for search operations.
  • Tests

    • Implemented unit tests to validate the functionality of the Usearch interface.
  • Version Update

    • Updated USEARCH_VERSION to 2.15.1, indicating new features and improvements.

@CLAassistant
Copy link

CLAassistant commented Sep 11, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

coderabbitai bot commented Sep 11, 2024

Walkthrough

Walkthrough

The changes introduce functionality for managing and installing the usearch library within a Makefile, which includes version management and installation targets for different operating systems. A new Go module dependency is added for usearch, and several new files are created to implement the usearch algorithm's API, configuration options, and error handling. Additionally, unit tests are provided to validate the Usearch functionality, and a version file is created to track the library's version.

Changes

File Path Change Summary
Makefile Added USEARCH_VERSION, version/usearch, and usearch/install targets for version management and installation.
dockers/ci/base/Dockerfile Included make usearch/install in the Docker build process to install the usearch library.
go.mod Added dependency for github.com/unum-cloud/usearch/golang with a specific version.
hack/actions/gen/main.go Introduced usearchVersionPath constant for versioning and added usearchPreprocess for build tasks.
internal/core/algorithm/usearch/option.go Created functional options for configuring the usearch algorithm, including validation logic.
internal/core/algorithm/usearch/usearch.go Implemented a Go API for managing a vector index with methods for saving, searching, and managing vectors.
internal/core/algorithm/usearch/usearch_test.go Added unit tests for the Usearch functionality, validating search operations and error handling.
internal/errors/usearch.go Defined a custom error type UsearchError for handling search-related errors.
versions/USEARCH_VERSION Introduced a new version 2.15.1 for the usearch library.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Makefile
    participant Docker
    participant GoModule
    participant UsearchAPI
    participant UsearchTests
    participant ErrorHandling

    User->>Makefile: Invoke make commands
    Makefile->>Docker: Build Docker image
    Docker->>Makefile: Execute usearch/install
    Makefile->>GoModule: Add dependency
    GoModule->>UsearchAPI: Initialize Usearch instance
    UsearchAPI->>UsearchTests: Run unit tests
    UsearchTests->>ErrorHandling: Validate error scenarios
Loading

Possibly related PRs

  • [Bugfix] agent service test #2620: The changes in this PR do not relate to the management or installation of the usearch library, as it focuses on a bugfix in a test file without any mention of usearch or related functionality.

Suggested labels

type/ci, size/L, area/agent/core/ngt


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@iammytoo iammytoo requested a review from hlts2 September 11, 2024 02:58
This commit fixes the style issues introduced in 58baee9 according to the output
from Gofumpt and Prettier.

Details: #2608
Copy link

cloudflare-workers-and-pages bot commented Sep 11, 2024

Deploying vald with  Cloudflare Pages  Cloudflare Pages

Latest commit: 644e80e
Status: ✅  Deploy successful!
Preview URL: https://5d4a3eb4.vald.pages.dev
Branch Preview URL: https://feature-agent-implement-usea.vald.pages.dev

View logs

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 51.61290% with 90 lines in your changes missing coverage. Please review.

Project coverage is 24.35%. Comparing base (5ba1375) to head (644e80e).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/core/algorithm/usearch/usearch.go 40.36% 56 Missing and 9 partials ⚠️
internal/core/algorithm/usearch/option.go 75.36% 10 Missing and 7 partials ⚠️
internal/errors/usearch.go 0.00% 5 Missing ⚠️
hack/docker/gen/main.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2608      +/-   ##
==========================================
+ Coverage   24.24%   24.35%   +0.11%     
==========================================
  Files         533      536       +3     
  Lines       46255    46439     +184     
==========================================
+ Hits        11213    11310      +97     
- Misses      34288    34361      +73     
- Partials      754      768      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

iammytoo and others added 3 commits September 12, 2024 10:55
This commit fixes the style issues introduced in 938cc12 according to the output
from Gofumpt and Prettier.

Details: #2608
@iammytoo iammytoo changed the title [WIP] feat: add usearch feat: add usearch Sep 12, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
internal/errors/usearch.go (1)

20-32: LGTM, but consider adding more context if needed.

The custom error type UsearchError, constructor function NewUsearchError, and Error method are correctly implemented and follow Go conventions.

Consider adding more context or structured fields to the UsearchError type if it would be helpful for error handling and debugging. For example:

type UsearchError struct {
    Msg string
    Code int
    Cause error
}
internal/core/algorithm/usearch/usearch_test.go (1)

1-379: LGTM! The test file is well-structured and comprehensive.

The code changes are approved. The test file follows best practices for unit testing in Go and covers a good range of scenarios to ensure the correctness of the Search method. The use of table-driven tests and helper functions makes the test code maintainable and readable.

Here are a few suggestions to further improve the test file:

  1. Consider adding more test cases to cover edge cases and boundary conditions, such as:

    • Searching with an empty query vector.
    • Searching with a large k value that exceeds the number of indexed vectors.
    • Searching with a negative k value.
    • Searching in an empty index.
  2. Consider extracting the common setup logic from the createFunc of each test case into a separate helper function. This will reduce duplication and make the test cases more focused on the specific scenario they are testing.

  3. Consider adding a test case to verify the behavior when the index file is not found or corrupted. This will help ensure graceful error handling in such scenarios.

  4. Consider adding a benchmark test to measure the performance of the Search method with different index sizes and query parameters. This will help track and optimize the performance of the usearch package over time.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c3c5765 and c7e9678.

Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
Files selected for processing (10)
  • Makefile (3 hunks)
  • dockers/ci/base/Dockerfile (1 hunks)
  • go.mod (1 hunks)
  • hack/actions/gen/main.go (1 hunks)
  • hack/docker/gen/main.go (3 hunks)
  • internal/core/algorithm/usearch/option.go (1 hunks)
  • internal/core/algorithm/usearch/usearch.go (1 hunks)
  • internal/core/algorithm/usearch/usearch_test.go (1 hunks)
  • internal/errors/usearch.go (1 hunks)
  • versions/USEARCH_VERSION (1 hunks)
Files skipped from review due to trivial changes (1)
  • versions/USEARCH_VERSION
Additional comments not posted (31)
internal/errors/usearch.go (2)

1-16: LGTM!

The Apache 2.0 license header is correctly included.


17-18: LGTM!

The package declaration and documentation are appropriate.

dockers/ci/base/Dockerfile (1)

123-123: LGTM!

The code change is approved. The addition of make usearch/install command to install the usearch library is consistent with the existing pattern and enhances the functionality of the Docker image.

internal/core/algorithm/usearch/option.go (7)

45-53: LGTM!

The code changes are approved.


110-120: LGTM!

The code changes are approved.


123-132: LGTM!

The code changes are approved.


135-144: LGTM!

The code changes are approved.


147-156: LGTM!

The code changes are approved.


159-164: LGTM!

The code changes are approved.


33-42: LGTM!

The code changes are approved.

internal/core/algorithm/usearch/usearch.go (14)

29-58: LGTM!

The Usearch interface looks good and covers all the necessary operations for managing the usearch index.


60-75: LGTM!

The usearch struct looks good and contains all the necessary fields for managing the usearch index.


78-81: LGTM!

The New function looks good and follows the common pattern for constructor functions.


83-85: LGTM!

The Load function looks good and follows the common pattern for constructor functions.


87-128: LGTM!

The gen function looks good and handles the initialization of the usearch index based on the isLoad flag.


130-140: LGTM!

The SaveIndex method looks good and ensures thread safety by acquiring a write lock.


142-152: LGTM!

The SaveIndexWithPath method looks good and ensures thread safety by acquiring a write lock.


154-163: LGTM!

The GetIndicesSize method looks good and ensures thread safety by acquiring a read lock.


165-178: LGTM!

The Add method looks good and ensures thread safety by acquiring a write lock. It also validates the dimension of the vector before adding it to the index.


180-189: LGTM!

The Reserve method looks good and ensures thread safety by acquiring a write lock.


191-211: LGTM!

The Search method looks good and ensures thread safety by acquiring a read lock. It also validates the dimension of the query vector before performing the search.


213-229: LGTM!

The GetObject method looks good and ensures thread safety by acquiring a read lock.


231-241: LGTM!

The Remove method looks good and ensures thread safety by acquiring a write lock.


243-251: LGTM!

The Close method looks good and properly frees the resources associated with the usearch index.

hack/actions/gen/main.go (1)

316-316: LGTM!

The addition of the usearchVersionPath constant is a straightforward and consistent extension of the existing versioning mechanism. It allows for tracking the version of the "usearch" component alongside other components like NGT and Faiss.

The code change is approved.

hack/docker/gen/main.go (3)

244-246: LGTM!

The addition of the usearchPreprocess constant follows the existing pattern for ngt and faiss preprocess constants. It indicates that a new usearch library is being introduced, and the assigned make command suggests that there will be a corresponding usearch/install target in the Makefile to handle the installation.


649-649: LGTM!

The addition of usearchPreprocess to the Preprocess slice of the vald-ci-container configuration ensures that the usearch library is installed as part of the preprocessing steps. This change is consistent with the existing preprocessing steps for ngt and faiss libraries.


667-668: LGTM!

The addition of usearchPreprocess to the Preprocess slice of the vald-dev-container configuration ensures that the usearch library is installed as part of the preprocessing steps for the development container. This change maintains consistency with the existing preprocessing steps for ngt and faiss libraries.

Makefile (2)

88-88: LGTM!

The code changes are approved. The new variable USEARCH_VERSION is defined correctly, following the existing pattern of reading the version from a file.


607-611: LGTM!

The code changes are approved. The new targets version/usearch and usearch/install are implemented correctly, following the existing pattern for managing dependencies. The installation process handles different operating systems appropriately.

Also applies to: 686-701

go.mod (1)

383-383: New dependency added: github.com/unum-cloud/usearch/golang

A new dependency on the usearch library has been added. A few suggestions:

  • Consider pointing to a specific release version instead of a commit hash for better reproducibility of builds.
  • Ensure that thorough testing is done to validate the integration of this new library and check for any regressions.
  • Update the documentation as needed to reflect this new dependency.

internal/core/algorithm/usearch/option.go Outdated Show resolved Hide resolved
internal/core/algorithm/usearch/option.go Outdated Show resolved Hide resolved
Copy link
Contributor

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@kpango
Copy link
Collaborator

kpango commented Sep 13, 2024

@all-contributors please add @iammytoo for code,research

Copy link
Contributor

@kpango

I've put up a pull request to add @iammytoo! 🎉

@kpango kpango merged commit e67dfa1 into main Sep 13, 2024
168 of 193 checks passed
@kpango kpango deleted the feature/agent/implement-usearch branch September 13, 2024 03:03
vdaas-ci pushed a commit that referenced this pull request Sep 13, 2024
* feat: add usearch

* style: format code with Gofumpt and Prettier

This commit fixes the style issues introduced in 58baee9 according to the output
from Gofumpt and Prettier.

Details: #2608

* feat: impl usearch istallation cmd for ci/base container

* style: format code with Gofumpt and Prettier

This commit fixes the style issues introduced in 938cc12 according to the output
from Gofumpt and Prettier.

Details: #2608

* add: multiple vector test

* fix: add ldconfg to Makefile

* refactor: covert switch to map

---------

Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
Co-authored-by: Hiroto Funakoshi <hiroto.funakoshi.hiroto@gmail.com>
Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>
Co-authored-by: Yusuke Kato <kpango@vdaas.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants