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

Generate code based on 2024-10 open api spec and add rerank #155

Closed
wants to merge 6 commits into from

Conversation

rohanshah18
Copy link
Contributor

@rohanshah18 rohanshah18 commented Oct 12, 2024

Problem

Add support for reranking documents.

Solution

Rerank is a part of 2024-10 openAPI spec, so the underlying code was regenerated based on the latest spec. Following are the important changes made as a part of this PR.

  1. Updated build-oas.sh to include all control plane, data plane, and inference modules.
  2. Inference is now broken out of control plane so the underlying Inference code generated by openAPI spec is now updated. This also resulted in instantiating a separate ApiClient which is a part of Inference module now instead of the shared ApiClient class of control plane. As a part of this refactoring, the customOkHttpClient is now a part of the PineconeConfig class, so once the customOkHttpClient is set by the user at the time of instantiating Pinecone using its builder inner class, it'll be stored in the PineconeConfig instance which can further be leveraged for Inference api calls as well.
  3. Control and data plane is now prepended with db_ so this resulted in updating import statements for a lot of existing classes.
  4. Added the following two methods for rerank:
    a. Method that accepts required parameters only and the optional parameters are set to default. More details on the api and its default parameters can be found here
      rerank(String model,
                       String query,
                       List<Map<String, String>> documents)

b. Method that accepts all parameters:

      rerank(String model,
                       String query,
                       List<Map<String, String>> documents,
                       List<String> rankFields,
                       int topN,
                       boolean returnDocuments,
                       Map<String, Object> parameters)
  1. Added integration tests for both rerank methods.
  2. Added an example in the README file.

Note: Everything under org/openapitools directory can be skipped as a part of the review process. All of the code under this directory was generated using openApi.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Added integration tests.

@rohanshah18 rohanshah18 marked this pull request as ready for review October 15, 2024 20:52
@@ -487,7 +487,7 @@ UpdateResponse updateResponse = index.update("v1", values, "example-namespace");

# Collections

Collections fall under data plane operations.
Collections fall under control plane operations.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typo from before

@@ -583,6 +583,66 @@ EmbeddingsList embeddings = inference.embed(embeddingModel, parameters, inputs);
List<Embedding> embeddedData = embeddings.getData();
```

## Rerank
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add example for rerank

@@ -3,7 +3,7 @@
set -eux -o pipefail

version=$1 # e.g. 2024-07
modules=("control")
modules=("db_control" "db_data" "inference")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. replaced control with db_control
  2. added db_data for bulk import (actual feature is going to be a part of the next PR).
  3. added inference which is now broken out of control plane

import io.pinecone.proto.DescribeIndexStatsResponse;
import io.pinecone.proto.NamespaceSummary;
import org.openapitools.control.client.model.*;
import org.openapitools.db_control.client.model.*;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

example of updating import statement since db_ is now appended.

@@ -4,8 +4,8 @@
import io.pinecone.clients.Pinecone;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.openapitools.control.client.ApiException;
import org.openapitools.control.client.model.EmbeddingsList;
import org.openapitools.inference.client.ApiException;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example of inference being its own module instead of being a part of the control plane which resulted in updating the import statement

private static final Inference inference = pinecone.getInferenceClient();

@Test
public void testRerank() throws ApiException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test all parameters for rerank

}

@Test
public void testRerankWithRequiredParameters() throws ApiException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test required parameters for rerank


import java.util.List;
import java.util.Map;
import io.pinecone.configs.PineconeConfig;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since Inference is broken out of control plane, the ApiClient for control plane can't be used as done before

* @return RerankResult containing the ranked documents and their scores.
* @throws ApiException If the API call fails, an ApiException is thrown.
*/
public RerankResult rerank(String model,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rerank with required params only

* @return RerankResult containing the ranked documents and their scores.
* @throws ApiException If the API call fails, an ApiException is thrown.
*/
public RerankResult rerank(String model,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rerank with required + optional params

@@ -1033,7 +1033,7 @@ public Builder withProxy(String proxyHost, int proxyPort) {
* @return A new {@link Pinecone} instance configured based on the builder parameters.
*/
public Pinecone build() {
PineconeConfig config = new PineconeConfig(apiKey, sourceTag, proxyConfig);
PineconeConfig config = new PineconeConfig(apiKey, sourceTag, proxyConfig, customOkHttpClient);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

customOkHttpClient being passed to the PineconeConfig constructor at the time of initializing PineconeConfig in Pinecone class

}

/**
* Constructs a {@link PineconeConfig} instance with the specified API key, source tag, control plane proxy
* configuration, and data plane proxy configuration.
* Constructs a {@link PineconeConfig} instance with the specified API key, source tag, a HTTP proxy configuration,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

customOkHttpClient is now a member of PineconeConfig. It makes a lot of sense since gRPC custom channel is already a part of this config class.

@rohanshah18 rohanshah18 changed the title Rerank Generate code based on 2024-10 open api spec and add rerank Oct 17, 2024
@rohanshah18 rohanshah18 deleted the rshah/rerank branch October 18, 2024 13:39
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.

1 participant