Embeddings correction #674
Conversation
|
This supports all types of models. Until we do not need more than one sequence or pooling we should use this code. |
| var embeddings = NativeApi.llama_get_embeddings_seq(Context.NativeHandle, LLamaSeqId.Zero); | ||
| if (embeddings.Length == 0) | ||
| return Array.Empty<float>(); | ||
| var embeddings = NativeApi.llama_get_embeddings(Context.NativeHandle); |
There was a problem hiding this comment.
In my own branch for investigating this issue I have rewritten the various get_embeddings methods to return a raw float*, instead of a Span<float> (just exposing the llama.cpp symbol directly, instead of wrapping it). Can I suggest making the same change in your PR?
Since null is a valid value for these functions to return a Span isn't really a suitable wrapper.
There was a problem hiding this comment.
Changing to float* is a lot of work because you need to change several functions... maybe it is better if you merge this PR and then change it as you like. Pointers are 'unsafe', so I am not sure if that is a good idea, but I trust that you know what you are doing.
There was a problem hiding this comment.
Sure, I'll have a look at that once this one is merged :)
This reverts commit 9c91fac.
|
Looks good, I'll merge it once the tests pass. Thanks for helping me investigate the embeddings issue and working on this PR. |
Maybe double check that |
|
The C++ side doesn't return a size, just a pointer to the first element. So it's either null or a valid embedding vector. |
No description provided.