Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RAG MVP #374

Open
wants to merge 7 commits into
base: sk/pg-vector
Choose a base branch
from
Open

Conversation

chidochipotle
Copy link

Adding a minimum minimum viable RAG feature

apps/service_providers/llm_service/runnables.py Outdated Show resolved Hide resolved
apps/experiments/views/experiment.py Outdated Show resolved Hide resolved
apps/service_providers/llm_service/runnables.py Outdated Show resolved Hide resolved
@snopoke snopoke changed the title Sk/pg vector RAG MVP May 16, 2024
Daniel Kornhauser added 3 commits May 22, 2024 17:24
available_variables should be added as "source_material" using form_data , but I could not figure out how to od it with form_data.
@chidochipotle
Copy link
Author

Hi Simon, added the commits address your previous comments.

Note the last one is unfinished, hope we can finish it in our meeting.
Unfinished tasks:

  • available_variables should be added as "source_material" using form_data , but I could not figure out how to do it with form_data.
  • A message hint should be added to the UI when files are attach to explain how to add the "context" and "input" variables. (Guess you will do this on your end)
  • Testing was minimal see screenshot below:
    image

Totally independently:
I don't remember if I reported the bug where there are issues if you attach a file before creating the experiment.
It may not exist anymore.
I had to create an experiment every time I tested, sometimes if I reused old experiments after changing the code there was issues such as getting no answer with the system got in a bad state and started to throw lots of logs.

Copy link
Collaborator

@snopoke snopoke left a comment

Choose a reason for hiding this comment

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

Looking good. A few small comments but nothing major.

def store_rag_embedding(self, experiment) -> None:
file_path = experiment.files.all().last().file.path
splits = load_rag_file(file_path)
embeddings_model = OpenAIEmbeddings()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use embeddings from LLM service

Suggested change
embeddings_model = OpenAIEmbeddings()
embeddings_model = experiment.get_llm_service().get_openai_embeddings()

PGVector.from_texts(splits, embeddings_model, None, experiment)


@shared_task(bind=True, base=TaskbadgerTask)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't need to be a task since it's just called as a normal function

Suggested change
@shared_task(bind=True, base=TaskbadgerTask)

@@ -361,6 +365,7 @@ def form_valid(self, form):
experiment = get_object_or_404(Experiment, team=self.request.team, pk=self.kwargs["pk"])
file = super().form_valid(form)
experiment.files.add(file)
store_rag_embedding(experiment)
Copy link
Collaborator

@snopoke snopoke May 23, 2024

Choose a reason for hiding this comment

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

To execute this as a task you should call .delay:

Suggested change
store_rag_embedding(experiment)
store_rag_embedding.delay(experiment.id)

Also we just pass the experiment ID to avoid serialising the whole experiment object

@@ -23,6 +27,44 @@ def get_response_for_webchat_task(self, experiment_session_id: int, message_text
return message_handler.new_user_message(message)


@shared_task(bind=True, base=TaskbadgerTask)
def store_rag_embedding(self, experiment) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than taking the experiment object this should take just the ID and fetch the experiment from the DB:

Suggested change
def store_rag_embedding(self, experiment) -> None:
def store_rag_embedding(self, experiment_id: int) -> None:
experiment = Experiment.objects.get(id=experiment_id)

@chidochipotle
Copy link
Author

@snopoke, just pushed your fixes.

Tested with a simple pdf file:
RAG_test_file.pdf

The following prompt:

You are an assistant for question-answering tasks. Use the following pieces of retrieved context to answer the question. If you don't know the answer, just say that you don't know. Use three sentences maximum and keep the answer concise.

Question: {input} 

Context: {context} 

Answer:

The following questions:

Hello, you can ask me anything you want about last_test.

What is "Policy objective 1" ?

Policy objective 1 is to reduce newborn, child, and adolescent morbidity and mortality due to preventable communicable diseases.

What is the capital of France ?

I don't know.

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.

None yet

2 participants