Supporting blog content - Local rag with lightweight Elasticsearch #488
Conversation
|
Review these changes at https://app.gitnotebooks.com/elastic/elasticsearch-labs/pull/488 |
There was a problem hiding this comment.
Can you amend the output to ask the LLM to include sources? This will make it easier for the audience to find the applicable document from the dataset.
There was a problem hiding this comment.
I added citations to the LLM’s responses!
There was a problem hiding this comment.
I'm slightly worried about including an example suggesting that a particular technology (specifically Elasticsearch) is slow, since this is on Elasticsearch labs. Especially since one fo the other models suggests it's inefficient. It might be worth amending the transcripts to include a common slowness use case (such as sharding or node issues for a high volume) and regenerate the answer. Alternatively I would change it to different technologies.
There was a problem hiding this comment.
You’re right, I will make sure to avoid mentioning specific names in the datasets. I removed those names and used generic ones; for example, I replaced Elasticsearch with “Database” and Redis with “Cache implementation.” Related commit.
|
|
||
|
|
||
| ## Stats | ||
| ✅ Indexed 5 documents in 250ms |
There was a problem hiding this comment.
Why is the indexing differing between models? I would expect indexing is a one-off operation independent to the model. This doesn't make sense to me. Should it be removed or clarified?
There was a problem hiding this comment.
I’m not sure why it differs between tests; I think it’s better to remove it. I did.
There was a problem hiding this comment.
I would change this example to something generic such as "Why is the sky blue?" or something else. As a developer this comes across as quite cringy to me.
There was a problem hiding this comment.
Changed. I added a new file showing the output response. Related commit.
| from openai import OpenAI | ||
|
|
||
| ES_URL = "http://localhost:9200" | ||
| ES_API_KEY = "your-api-key-here" |
There was a problem hiding this comment.
I would change the URL, API key and LOCAL_AI_URL values to environment variables that are loaded via something like dotenv and a local .env file. While this is fine for local development, when developers try to move this to production they need to tidy it up. So lets set the example now.
There was a problem hiding this comment.
Ok! Added dotenv support! Related commit.
| ai_client = OpenAI(base_url=LOCAL_AI_URL, api_key="sk-x") | ||
|
|
||
|
|
||
| def build_documents(dataset_folder, index_name): |
There was a problem hiding this comment.
Should this be called load_documents as it's opening text files. It's not really building the documents from scratch which is misleading.
There was a problem hiding this comment.
Done! Method renamed!
| if filename.endswith(".txt"): | ||
| filepath = os.path.join(dataset_folder, filename) | ||
|
|
||
| with open(filepath, "r", encoding="utf-8") as file: |
There was a problem hiding this comment.
I would add a comment explaining why you've used utf-8 encoding here.
| } | ||
|
|
||
|
|
||
| def index_documents(): |
There was a problem hiding this comment.
Add top level comments for each function explaining what they do.
There was a problem hiding this comment.
Added functions descriptions! Related commit.
| start_time = time.time() | ||
|
|
||
| try: | ||
| response = ai_client.chat.completions.create( |
There was a problem hiding this comment.
I would perhaps add a comment making clear that this is a simple generate rather than streaming of the response token by token.
There was a problem hiding this comment.
Added a clarifying comment!
| try: | ||
| start_time = time.time() | ||
|
|
||
| success, _ = helpers.bulk( |
There was a problem hiding this comment.
You should add the index creation code either here based on the condition that the index doesn't exist, or in a separate utility function. For semantic text you'll need to specify that mapping when creating the index, and that step is missing here.
There was a problem hiding this comment.
I added the index creation as a method in the script. I also added a verification step before bulk-indexing the data.
ulations
2 participants
No description provided.