Skip to content

Add custom model#463

Closed
I8dNLo wants to merge 4 commits into
mainfrom
support_custom_model
Closed

Add custom model#463
I8dNLo wants to merge 4 commits into
mainfrom
support_custom_model

Conversation

@I8dNLo

@I8dNLo I8dNLo commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

Add custom models to fastembed.text

@I8dNLo I8dNLo requested a review from joein February 4, 2025 13:10
@I8dNLo I8dNLo changed the title add_custom_model draft Add custom model Feb 4, 2025
Comment on lines +108 to +110
@classmethod
def add_custom_model(cls, model_info: dict[str, Any]):
cls.supported_models.append(model_info)

@joein joein Feb 10, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there is no need to redefine this method in the subclasses if they fetch the right cls variable

"description": "...",
"license": "apache-2.0",
"size_in_GB": 1.23,
"sources": { ... } # optional

@hh-space-invader hh-space-invader Feb 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't that be mandatory ? We use sources to download model/config.

Suggested change
"sources": { ... } # optional
"sources": {
"hf": "alibaba/blablabla"
}

PooledEmbedding.add_custom_model(model_info)
elif mean_pooling and normalization:
PooledNormalizedEmbedding.add_custom_model(model_info)
elif "clip" in model_info["model"].lower():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not always the case. jinaai/jina-clip-v1 model is added to text models (not clip) as it needs normalization.

Suggested change
elif "clip" in model_info["model"].lower():
elif "clip" in model_info["model"].lower() and not normalization:

@joein

joein commented Feb 20, 2025

Copy link
Copy Markdown
Member

closed in favour of #479

@joein joein closed this Feb 20, 2025
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