Skip to content

Conversation

@KanonWY
Copy link

@KanonWY KanonWY commented Dec 29, 2025

use ollama embedding_model, Missing model dimensions.

    let ollama_client = ollama::Client::from_env();
   // this will panic.
    let model = ollama_client.embedding_model("nomic-embed-text");

add function model_dimensions_from_identifier to give a default dimension.
maybe fix 1189

Copy link
Collaborator

@joshua-mo-143 joshua-mo-143 left a comment

Choose a reason for hiding this comment

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

Looks OK. See my comment

Comment on lines +150 to +157
fn model_dimensions_from_identifier(identifier: &str) -> Option<usize> {
match identifier {
ALL_MINILM => Some(384),
NOMIC_EMBED_TEXT => Some(768),
_ => None,
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this part is largely unmaintainable, so we should avoid adding this in. It is usually quite trivial to find the dimension sizes

Copy link
Author

Choose a reason for hiding this comment

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

yes, I think this part is largely unmaintainable too. A better approach is to use embedding_model_with_ndims. I added it this way because I noticed that other modules did the same. openai/embedding.rs:row59

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.

bug: Panic caused by Option::Unwrap() called on None value in Ollama Provider EmbeddingModel

2 participants