Skip to content

Conversation

voorhs
Copy link
Collaborator

@voorhs voorhs commented Sep 3, 2025

No description provided.

@voorhs voorhs marked this pull request as ready for review September 9, 2025 07:19
@voorhs voorhs requested a review from Samoed September 9, 2025 07:19
@voorhs
Copy link
Collaborator Author

voorhs commented Sep 9, 2025

сейчас мне немного разонравилось то как устроены dump и load у Embedder и VectorIndex и в целом конфиги для них - по-моему в таком варианте нельзя задать конфиг из yaml файла

я это поправлю в другом PR

"""
return self._backend.get_hash()

def train(self, utterances: list[str], labels: ListOfLabels, config: EmbedderFineTuningConfig) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Мб в BaseEmbeddingBackend добавить train и его вызывать?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

не думаю что красивее сделать получится

тут потому что EmbedderFineTuningConfig содержит специфичные для sentence-transformers параметры

я бы тогда просто предложил переименовать train -> train_st и в докстринге обозначить что этот метод работает только с SentenceTransformersEmbeddingBackend

все равно пока что фича с тренировкой эмбедингов не встроена в наш EmbeddingNode

batch = utterances[i : i + self.config.batch_size]

# Prepare API call parameters
kwargs: EmbeddingsCreateKwargs = {
Copy link
Member

Choose a reason for hiding this comment

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

Можно добавить truncate_prompt_tokens, не знаю есть такое у openai или нет (но на сколько помню у них вообще криво все сделано по токенизации)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

не очень вообще шарю за то, что за truncate_prompt_tokens

можешь подробнее рассказать в будущем или открыть issue?

Copy link
Member

Choose a reason for hiding this comment

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

У vllm есть параметр truncate_prompt_tokens, который будет обрезать ввод по количеству токенов, если больше max_tokens модели отправить, то выдается ошибка. Если в openai отправить больше чем 8191 токен, то тоже просто ошибка будет

Comment on lines +224 to +229
kwargs: EmbeddingsCreateKwargs = {
"input": batch,
"model": self.config.model_name,
}
if self.config.dimensions is not None:
kwargs["dimensions"] = self.config.dimensions
Copy link
Member

Choose a reason for hiding this comment

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

Можно вынести в функцию

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

в аргументы функции _process_embeddings_sync?

Copy link
Member

Choose a reason for hiding this comment

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

Создание kwargs можно вынести в функцию, тк дублируется между функциями

@voorhs voorhs requested a review from Samoed September 20, 2025 11:17
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.

2 participants