Skip to content

Conversation

@vdaita
Copy link
Collaborator

@vdaita vdaita commented Jun 12, 2024

No description provided.

@vdaita vdaita requested review from JialeTomTian and ganler June 12, 2024 19:11
Copy link
Member

@ganler ganler left a comment

Choose a reason for hiding this comment

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

did a quick review. will check details again later

from typing import List, Tuple

from openai import Client
from sklearn.metrics.pairwise import cosine_similarity
Copy link
Member

Choose a reason for hiding this comment

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

sklearn is a bit huge. ideally we want some lighter dependencies (avoiding the risk of dep conflicts in the future) or just impl the function ourselves.

trust_remote_code: bool = False,
attn_implementation=None,
is_embedding: bool = False,
embedding_context_chunk_size: int = 30
Copy link
Member

Choose a reason for hiding this comment

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

is this # of chunks or # tokens for each chunk? better make it more explicit :)
also i think specifying the size /#token of each chunk might be more intuitive

Copy link
Collaborator Author

@vdaita vdaita Jun 12, 2024

Choose a reason for hiding this comment

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

Currently it's lines of code per chunk, will change the variable name

@vdaita vdaita requested a review from ganler June 23, 2024 18:13
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.

3 participants