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

BUG: fix ImportError when optional dependency FlagEmbedding is not installed #2649

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

zjuyzj
Copy link
Contributor

@zjuyzj zjuyzj commented Dec 10, 2024

#2540 introduces FlagEmbedding package to support sparse vector for bge-m3. However, FlagEmbedding is an optional dependency for embedding model (but required for rerank):

inference/setup.cfg

Lines 184 to 187 in 70a0081

embedding =
sentence-transformers>=3.1.0
rerank =
FlagEmbedding

In the case FlagEmbedding is not installed, an ImportError will be raised even if embedding models only based on sentence-transformers are used, due to the logic related to BGEM3FlagModel. This PR fixes this problem.

By the way, maybe adding FlagEmbedding to required dependency for embedding is not suitable now until upstream solve the dependency, since it requires transformers==4.44.2(https://github.com/FlagOpen/FlagEmbedding/blob/master/setup.py#L18) and will make models like Qwen2-VL unusable (https://huggingface.co/Qwen/Qwen2-VL-7B-Instruct/discussions/5).

@XprobeBot XprobeBot added the bug Something isn't working label Dec 10, 2024
@XprobeBot XprobeBot added this to the v1.x milestone Dec 10, 2024
@qinxuye
Copy link
Contributor

qinxuye commented Dec 10, 2024

Tests seem failed.

@zjuyzj zjuyzj marked this pull request as draft December 10, 2024 09:08
…onal dependency FlagEmbedding is not installed.
Copy link
Contributor

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants