Add embedding cli #253
Add embedding cli #253
Conversation
Signed-off-by: Abhi Jain <devabhixda@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new embed command to the CLI that enables users to generate text embeddings using embedding models. The implementation follows the same patterns as the existing chat command.
Key Changes
- Adds a new C++ program (
embed.cpp) that calls thecactus_embedFFI function to generate embeddings - Extends the shell script to support building and running the embed command with flexible model selection
- Updates documentation with usage examples
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cli/embed.cpp | New C++ program that initializes a model, generates embeddings for input text, and outputs them as a JSON array |
| cli/cactus | Adds build_embed() and run_embed() functions to compile and execute the embed program; includes help text and command routing for the new embed command |
| README.md | Documents the new embed command with an example showing how to generate embeddings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
|
|
||
| if (result < 0) { | ||
| std::cerr << "Error: Failed to generate embedding (error code: " << result << ")\n"; |
There was a problem hiding this comment.
The error code -2 indicates buffer overflow but this is not handled distinctly from other errors. Consider providing a more specific error message for buffer overflow (error code -2) to help users understand they need a larger buffer.
| std::cerr << "Error: Failed to generate embedding (error code: " << result << ")\n"; | |
| if (result == -2) { | |
| std::cerr << "Error: Buffer overflow while generating embedding (error code: -2). Try increasing the buffer size (MAX_EMBEDDING_DIM).\n"; | |
| } else { | |
| std::cerr << "Error: Failed to generate embedding (error code: " << result << ")\n"; | |
| } |
| #include <iomanip> | ||
|
|
There was a problem hiding this comment.
The iomanip header is included but never used in this file. Consider removing this unused include.
| #include <iomanip> |
| std::cout << "["; | ||
| for (size_t i = 0; i < embedding_dim; i++) { | ||
| if (i > 0) std::cout << ","; | ||
| std::cout << embeddings[i]; | ||
| } |
There was a problem hiding this comment.
The floating-point values are output without specifying precision, which may result in inconsistent or truncated output depending on the default stream precision. Consider using std::setprecision or std::scientific to ensure consistent and accurate representation of embedding values.
| int main(int argc, char* argv[]) { | ||
| if (argc != 3) { | ||
| std::cerr << "Usage: " << argv[0] << " <model_path> <text>\n"; | ||
| std::cerr << "Example: " << argv[0] << " weights/lfm2-1.2B \"Hello world\"\n"; |
There was a problem hiding this comment.
The example shows "weights/lfm2-1.2B" but based on the get_model_dir_name function in the cactus script, the model directory name would be all lowercase ("lfm2-1.2b"). This example should use all lowercase to match the actual directory naming convention.
| std::cerr << "Example: " << argv[0] << " weights/lfm2-1.2B \"Hello world\"\n"; | |
| std::cerr << "Example: " << argv[0] << " weights/lfm2-1.2b \"Hello world\"\n"; |
No description provided.