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

[vLLM] metadata script #959

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[vLLM] metadata script #959

wants to merge 2 commits into from

Conversation

mishig25
Copy link
Collaborator

@mishig25 mishig25 commented Oct 8, 2024

Follow up to #957. To precisely show vLLM snippet on HF models, we need to have record on which models are supported by vLLM. vLLM provides supported models list: doc page, python src.

image

This PR creates a github cron job (runs once a day) script that:

  1. parses python src through AST to extract supported architectures list (both for transformers & ggml/llama.cpp styles)
  2. uploads the result to huggingface/vllm-metadata. You can see working example here.
image
  1. HF models will use this list to whether show or not show vLLM snippet.

Find the python script here: get_vlmm_metadata.py.zip (you can locally test run if you want).

@mishig25 mishig25 changed the title vlmm_metadata [vLLM] metadata script Oct 8, 2024
@mishig25
Copy link
Collaborator Author

mishig25 commented Oct 8, 2024

@simon-mo could you confirm that this list is the correct way to find vLLM supported models both for transformers & ggml/llama.cpp architectures

_MODELS = {
    **_TEXT_GENERATION_MODELS,
    **_EMBEDDING_MODELS,
    **_MULTIMODAL_MODELS,
    **_SPECULATIVE_DECODING_MODELS,
}

@simon-mo
Copy link

simon-mo commented Oct 8, 2024

Yes!

Copy link
Member

@Vaibhavs10 Vaibhavs10 left a comment

Choose a reason for hiding this comment

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

Looks dope! I didn't test the script locally, but went through the exported dataset - looks amazing!

source_code = response.text

models_dict = extract_models_dict(source_code)
architectures = [item for tup in models_dict.values() for item in tup]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
architectures = [item for tup in models_dict.values() for item in tup]
architectures = sorted(list({item for tup in models_dict.values() for item in tup}))

Maybe, if we want to remove duplicates and assuming tuple order does not matter (i.e., llama does not have to appear before LlamaForCausalLM

Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Very cool 🔥

- name: Execute Python script
env:
HF_VLLM_METADATA_PUSH: ${{ secrets.HF_VLLM_METADATA_PUSH }}
run: |
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this code to a script rather than having the Python code in the yaml? It will be easier to maintain, update, and review

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it will be easier to review

agree with this point

it will be easier to maintain, update

I think maintaining a separate python script would be painful. We would need to find a place to place this python script and tell the yaml job to download and run this python script (which can introduce other security issues since we are running what's being downloaded)

from huggingface_hub import HfApi

def extract_models_sub_dict(parsed_code, sub_dict_name):
class MODELS_SUB_LIST_VISITOR(ast.NodeVisitor):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class MODELS_SUB_LIST_VISITOR(ast.NodeVisitor):
class ModelsSubListVisitor(ast.NodeVisitor):

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.

5 participants