Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

Add Anyscale Endpoint support and Llama Tokenizer #173

Merged
merged 18 commits into from
Nov 27, 2023

Conversation

kylehh
Copy link
Contributor

@kylehh kylehh commented Nov 14, 2023

Problem

  1. No support for open source models
  2. No support for open source tokenizers

Solution

  1. Add Anyscale Endpoint LLM client which hosts Llama2 , Mistrial and Zephyr models.
  2. Add Llama Tokenizer based on huggingface's tokenizers library

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] 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

Describe specific steps for validating this change.

  1. Get your HF and Anyscale tokens and add them in the config/anyscale.yaml
  2. canopy start --config anyscale.yaml

Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

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

@kylehh Thank you very much for your contribution!! This is highly appreciated!

Please see a few comments. The main 3 issues:

  1. We plan to merge Upgrade openai sdk to v.1.2.3 #171 today, which adds a base_url init argument to OpenAILLM class. With that change, the implementation of AnyscaleLLM could be as easy as inheriting OpenAILLM and passing hard coded base_url to super().__init__().
  2. The new classes, AnyscaleLLM and LlamaTokenizer require their own unit classes before this PR can be merged.
  3. Let's figure out together if it's really required for every Canopy user to generate a HF identification token just for the Tokenizer, or can we find a more user friendly solution.

config/anyscale.yaml Show resolved Hide resolved
config/anyscale.yaml Outdated Show resolved Hide resolved
config/anyscale.yaml Outdated Show resolved Hide resolved
src/canopy/llm/anyscale.py Outdated Show resolved Hide resolved
src/canopy/llm/anyscale.py Outdated Show resolved Hide resolved
src/canopy/tokenizer/llama.py Show resolved Hide resolved
src/canopy/llm/anyscale.py Show resolved Hide resolved
src/canopy/tokenizer/llama.py Show resolved Hide resolved
src/canopy/tokenizer/llama.py Outdated Show resolved Hide resolved
src/canopy/utils/openai_exceptions.py Outdated Show resolved Hide resolved
kylehh and others added 6 commits November 16, 2023 08:48
The underlying HuggingFace tokenizer has a proper `.tokenize()` method, as well as a `.convert_tokens_to_string()` method. Together with a few configurations like `add_bos_token=False` the tokenizer now passes all tests
Added a few more asserts, and more elaborate prints on failure
The new LlamaTokenizer class should be almost identical to OpenAI tokenizer, other than the expected tokens being slightly different
Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

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

HI @kylehh,

As we discussed over slack - there are some changes needed for the Tokenizer, please see suggested changes in kylehh#1.

In addition, there a few more issues to solve, namely the unit test for AnyscaleLLM and the linter tests

src/canopy/llm/anyscale.py Outdated Show resolved Hide resolved
src/canopy/llm/anyscale.py Outdated Show resolved Hide resolved
src/canopy/llm/anyscale.py Outdated Show resolved Hide resolved
config/anyscale.yaml Outdated Show resolved Hide resolved
src/canopy/llm/anyscale.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
src/canopy/tokenizer/llama.py Outdated Show resolved Hide resolved
tests/system/llm/test_anyscale.py Outdated Show resolved Hide resolved
src/canopy/llm/anyscale.py Show resolved Hide resolved
Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

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

LGTM!

Thank you @kylehh for all of your efforts. Merging.

@igiloh-pinecone igiloh-pinecone added this pull request to the merge queue Nov 26, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 26, 2023
@igiloh-pinecone igiloh-pinecone added this pull request to the merge queue Nov 26, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 26, 2023
@igiloh-pinecone igiloh-pinecone added this pull request to the merge queue Nov 26, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 26, 2023
@igiloh-pinecone igiloh-pinecone added this pull request to the merge queue Nov 27, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 27, 2023
@igiloh-pinecone igiloh-pinecone added this pull request to the merge queue Nov 27, 2023
Merged via the queue into pinecone-io:main with commit 63c2da4 Nov 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants