llama integration with exception safety#2
Conversation
- changes made in docwire.cpp for default installation purpose.
1. Added configurable model load and unload feature, which gives sdk an option to decide whether to unload to the model after pipeline usage or keep it persistent for next usage. 2. Added files for local summarize and translate
api_tests.cpp for GitHub Build
Adjustment in local_ai.cmake for the same flags tests/CMakeLists.txt gets a minimal conditional added
| if (vm.count("local-ai-prompt") || vm.count("local-ai-embed")) | ||
| { | ||
| std::cerr << "Error: Local AI features requested, but this build does not include " | ||
| "DOCWIRE_LOCAL_CT2 support.\n" |
There was a problem hiding this comment.
It's a little bit counter-intuitive that we need ct2 to use --local-ai-prompt if we have llama.cpp engine enabled. Maybe this option should work with llama.cpp as well?
|
|
||
| add_library(docwire_ai_ct2 SHARED ct2_runner.cpp tokenizer.cpp) | ||
|
|
||
| target_compile_definitions(docwire_ai_ct2 PUBLIC DOCWIRE_LOCAL_CT2) |
There was a problem hiding this comment.
Do we need this? It will always be true i think.
as-ascii
left a comment
as-ascii
left a comment
There was a problem hiding this comment.
Changes are good in general but not complete yet I think.
| * The appropriate prefix for the underlying model (e.g. "passage: " for multilingual-e5-small) | ||
| * is applied automatically. No model-specific knowledge required at the call site. | ||
| */ | ||
| class DOCWIRE_LOCAL_AI_EXPORT embed_passage : public docwire::ai::embed |
There was a problem hiding this comment.
We need to think about best name. "passage" is not well known in developers community, only for AI experts. I propose "document" or "content" or "index". And maybe embed::query not embed_query? But the concept of two classes instead of enum argument is interesting. Probably shorter: embed::query{} instead of embed{embed::query} so nicer in examples.
There was a problem hiding this comment.
Current namespace is: docwire::ai::local, may be that needs to be changed to docwire::ai::local:embed and then the two classes. But, is it required? an additional nesting in namespace?
There was a problem hiding this comment.
You are doing it now but in old C style: embed_X, embed_Y ;-) Its kind of a namespace in my opinion, no? Do you see any cons of converting it to embed::X and embed::Y? I have other doubts about naming: "embed" suggests a function rather than class/object, maybe it should be named "embedder" or "vectorizer"? Maybe it should be query::embedder and index::vectorizer ? Naming is hard for creators but very important for users.
There was a problem hiding this comment.
I think that we can follow content_type namespace a little bit, for example index::embedder and query::embedder and then there is a possiblity to add for example index::embed and query::embed functions if user is not building a parsing chain just wanted to use function. I will try to add this kind of things into our "design rules" that are created currently.
There was a problem hiding this comment.
So, these index::embed and query::embed functions will be separate from usual constructors?
So proposed namespaces can be: docwire::ai:embedder::passage and docwire::ai:embedder::query.
Even content_type namespace, as I see, has main classification name first, and then various iteration of utilities like: docwire::content_type::by_signature, docwire::content_type::asp, docwire::content_type::by_file_extension
There was a problem hiding this comment.
Yes but for example by_signature is a namespace and there is class there "detector" and function "detect". If you will stay with class/object API for now than embedder::index and embedder::query have sense of course but "index" and "query" classes does not look like something that generate index or query but something that is index or query. Naming is hard... "embedder" looks like object that generates embeddings.
2 participants
Features worked: