add single execution option for ConversationalRetrievalChain #5066

jpzhangvincent · 2023-05-21T18:00:45Z

add single execution option for ConversationalRetrievalChain

The current default workflow for ConversationalRetrievalChain is to rephrase the question based on the chat history and then use the rephrased question to perform retrieval and question answering.
This current two-step process has pros and cons. It helps address the context size limit but it could increase unnecessary latency and incur potential information loss.
We can make the question_generator (chain) argument optional to allow it to bypass the rephrasing step and use a custom prompt to combine the chat_history, context and question together for single execution. This could give users more flexibility based on their use cases and requirements.

Before submitting

I updated the notebook to include the ## ConversationalRetrievalChain with only custom combine_docs_chain section

Who can review?

Community members can review the PR once tests pass. Tag maintainers/contributors who might be interested:
@hwchase17 @dev2049

jpzhangvincent · 2023-05-26T17:12:17Z

I believe this feature provides practical values. Can I get a review on this one? @hwchase17 @dev2049

franco-roura · 2023-05-29T13:12:57Z

Hello guys, we're having a similar issue with the ConversationalRetrievalQAChain class.
When adding a Pinecone vector store for memory, it can even reach 45 seconds of total response time, which is far from ideal. This PR does seem to have the potential to mitigate the issue, is it being actively looked at?

Thanks in advance.

dev2049

making question_generator optional makes sense to me! but don't think we should be messing with prompts at runtime, you should be able to set those when instantiating the chain in first place

dev2049 · 2023-05-29T18:24:17Z

langchain/chains/conversational_retrieval/base.py

+            and "context"
+            not in self.combine_docs_chain.llm_chain.prompt.input_variables
+        ):
+            self.combine_docs_chain.llm_chain.prompt = CHAT_RETRIEVAL_QA_PROMPT


why do this at runtime, why not set the prompt of combine_docs_chain when instantiating the ConversationalRetrievalChain?

That makes sense! Just made the change.

@dev2049 can you review again?

hwchase17 · 2023-06-03T21:43:29Z

i think we need just better documentation around conversational retrieval chains in general

jpzhangvincent · 2023-06-06T18:27:52Z

i think we need just better documentation around conversational retrieval chains in general

I have updated the notebook to show how to set question_generator as None and use a custom prompt. But feel free to let me know anything else needed to get this PR merged. Thanks!

jpzhangvincent · 2023-06-11T17:49:08Z

Hoping this can get merged soon since I need this patch for a work project! @hwchase17 @dev2049 any feedback?

italojs · 2023-06-13T23:16:56Z

👀👀👀

vercel · 2023-06-27T16:15:16Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
langchain	⬜️ Ignored (Inspect)	Visit Preview		Mar 6, 2024 0:39am

jpzhangvincent · 2023-06-27T16:59:23Z

@dev2049 @hwchase17 can you take a look at this PR? Sorry for chasing..

langchain/chains/conversational_retrieval/prompts.py

langchain/chains/conversational_retrieval/base.py

leferradkw · 2023-06-30T14:55:35Z

langchain/chains/combine_documents/base.py

@@ -36,6 +37,7 @@ def format_document(doc: Document, prompt: BasePromptTemplate) -> str:
 class BaseCombineDocumentsChain(Chain, ABC):
    """Base interface for chains combining documents."""

+    llm_chain: LLMChain


is this necessary?

Yes because we set the prompt variable like
combine_docs_chain.llm_chain.prompt = CHAT_RETRIEVAL_QA_PROMPT in the validate_combine_docs_chain function for the error handling and I found I have to put the type there to pass the linting check.

@leferradkw let me know if any other questions! Hope to get the PR merged soon! :)

Then add a unit test to prove the constructor is working well and the interface is the same as before

anthonycoded · 2023-07-01T18:59:32Z

How can I implement this solution in my project?

langchain/chains/combine_documents/base.py

leferradkw · 2023-07-10T12:47:31Z

langchain/chains/combine_documents/base.py

@@ -36,6 +37,7 @@ def format_document(doc: Document, prompt: BasePromptTemplate) -> str:
 class BaseCombineDocumentsChain(Chain, ABC):
    """Base interface for chains combining documents."""

+    llm_chain: LLMChain


Then add a unit test to prove the constructor is working well and the interface is the same as before

kowsikgelli · 2023-08-31T06:55:42Z

A must needed feature. I am looking for this.

leo-gan · 2023-09-13T20:25:25Z

@jpzhangvincent Hi , could you, please, resolve the merging issues and address the last comments (if needed)? After that, ping me and I push this PR for the review. Thanks!

leo-gan · 2023-09-15T01:46:43Z

@jpzhangvincent It is again :( Could you, please, resolve the merging issues? After that ping me and I push this PR for the review. Thanks!

jpzhangvincent · 2023-09-15T17:38:39Z

@jpzhangvincent It is again :( Could you, please, resolve the merging issues? After that ping me and I push this PR for the review. Thanks!

@leo-gan I believe I have fixed the merge conflicts. I tried to add a test or an example in the notebook(if needed) but not sure where/which file to add specifically. Open to suggestion!

leo-gan · 2023-09-15T18:30:50Z

@jpzhangvincent There are failed unit tests and pydantic compatibility tests. When you fix them we will be ready.

jpzhangvincent · 2023-09-19T06:03:28Z

@leo-gan I believe I had resolved the test issues.. please review again and feel free to make code changes directly!

jpzhangvincent · 2023-12-23T03:33:18Z

@baskaryan @leo-gan Okay I have addressed the feedback and added a unit test. Hopeful we can get this merged soon since it could be low-hanging fruit but useful feature.

jpzhangvincent · 2024-01-05T19:57:39Z

@baskaryan @leo-gan Okay I have addressed the feedback and added a unit test. Hopeful we can get this merged soon since it could be low-hanging fruit but useful feature.

@baskaryan @leo-gan @hwchase17 thoughts on merging this PR?

jpzhangvincent · 2024-03-06T00:40:43Z

Can you please review again? Thanks! @baskaryan @leo-gan

ccurme · 2024-07-08T13:57:50Z

Thank you for this. Closing because this is readily achievable with create_retrieval_chain. See the example snippet in the API reference. You can also reference this guide on migrating from ConversationalRetrievalChain to LCEL implementations. Please let me know if you have any other concerns.

jpzhangvincent mentioned this pull request May 23, 2023

Slow response time with ConversationalRetrievalQAChain #5123

Closed

dev2049 reviewed May 29, 2023

View reviewed changes

jpzhangvincent requested a review from dev2049 May 30, 2023 07:00

jpzhangvincent force-pushed the add-convQARetrchain-single-run branch from db0f390 to fd0153e Compare May 30, 2023 16:17

jpzhangvincent force-pushed the add-convQARetrchain-single-run branch from b92120d to 98794d1 Compare June 27, 2023 16:15

leferradkw reviewed Jun 30, 2023

View reviewed changes

jpzhangvincent force-pushed the add-convQARetrchain-single-run branch from 98794d1 to c496d5b Compare July 8, 2023 16:46

jpzhangvincent requested a review from leferradkw July 8, 2023 16:57

leferradkw reviewed Jul 10, 2023

View reviewed changes

dosubot bot added the 🤖:improvement Medium size change to existing code to handle new use-cases label Jul 14, 2023

jpzhangvincent force-pushed the add-convQARetrchain-single-run branch from c496d5b to 2f42514 Compare August 9, 2023 23:48

jpzhangvincent force-pushed the add-convQARetrchain-single-run branch from 2f42514 to 929e3c0 Compare September 14, 2023 22:57

vercel bot deployed to Preview September 14, 2023 23:15 View deployment

jpzhangvincent force-pushed the add-convQARetrchain-single-run branch from b33137d to 4418657 Compare September 18, 2023 21:59

leo-gan requested a review from baskaryan September 19, 2023 15:17

vercel bot deployed to Preview December 22, 2023 20:29 View deployment

jpzhangvincent added 2 commits December 22, 2023 12:58

revert notebook changes

ad60626

revert notebook to be same with master

74453d1

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Dec 22, 2023

vercel bot deployed to Preview December 22, 2023 21:09 View deployment

jpzhangvincent added 2 commits December 22, 2023 13:26

fix ruff errors

0624105

fix previous changes

cba48df

vercel bot deployed to Preview December 22, 2023 21:39 View deployment

add unit test

752348d

dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Dec 23, 2023

jpzhangvincent requested review from leferradkw and dev2049 December 23, 2023 03:31

vercel bot deployed to Preview December 23, 2023 03:37 View deployment

jpzhangvincent added 2 commits December 22, 2023 19:42

fix formatting

fd5e20f

fix test file formatting

5e9731f

vercel bot deployed to Preview December 23, 2023 03:54 View deployment

jpzhangvincent added 3 commits January 2, 2024 10:27

Merge branch 'master' into add-convQARetrchain-single-run

cf565f3

fix pytest error

e3faf7a

Merge branch 'master' into add-convQARetrchain-single-run

3b6bb27

hwchase17 closed this Jan 30, 2024

baskaryan reopened this Jan 30, 2024

Merge branch 'master' into add-convQARetrchain-single-run

ddf0f13

ccurme added the langchain Related to the langchain package label Jun 21, 2024

ccurme closed this Jul 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add single execution option for ConversationalRetrievalChain #5066

add single execution option for ConversationalRetrievalChain #5066

jpzhangvincent commented May 21, 2023 •

edited

Loading

jpzhangvincent commented May 26, 2023

franco-roura commented May 29, 2023

dev2049 left a comment

dev2049 May 29, 2023

jpzhangvincent May 30, 2023

jpzhangvincent Jun 3, 2023

hwchase17 commented Jun 3, 2023

jpzhangvincent commented Jun 6, 2023

jpzhangvincent commented Jun 11, 2023

italojs commented Jun 13, 2023

vercel bot commented Jun 27, 2023 •

edited

Loading

jpzhangvincent commented Jun 27, 2023

leferradkw Jun 30, 2023

jpzhangvincent Jul 8, 2023

jpzhangvincent Jul 8, 2023

leferradkw Jul 10, 2023

anthonycoded commented Jul 1, 2023

leferradkw Jul 10, 2023

kowsikgelli commented Aug 31, 2023

leo-gan commented Sep 13, 2023

leo-gan commented Sep 15, 2023

jpzhangvincent commented Sep 15, 2023

leo-gan commented Sep 15, 2023

jpzhangvincent commented Sep 19, 2023

jpzhangvincent commented Dec 23, 2023

jpzhangvincent commented Jan 5, 2024

jpzhangvincent commented Mar 6, 2024

ccurme commented Jul 8, 2024

add single execution option for ConversationalRetrievalChain #5066

add single execution option for ConversationalRetrievalChain #5066

Conversation

jpzhangvincent commented May 21, 2023 • edited Loading

add single execution option for ConversationalRetrievalChain

Before submitting

Who can review?

jpzhangvincent commented May 26, 2023

franco-roura commented May 29, 2023

dev2049 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwchase17 commented Jun 3, 2023

jpzhangvincent commented Jun 6, 2023

jpzhangvincent commented Jun 11, 2023

italojs commented Jun 13, 2023

vercel bot commented Jun 27, 2023 • edited Loading

jpzhangvincent commented Jun 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthonycoded commented Jul 1, 2023

Choose a reason for hiding this comment

kowsikgelli commented Aug 31, 2023

leo-gan commented Sep 13, 2023

leo-gan commented Sep 15, 2023

jpzhangvincent commented Sep 15, 2023

leo-gan commented Sep 15, 2023

jpzhangvincent commented Sep 19, 2023

jpzhangvincent commented Dec 23, 2023

jpzhangvincent commented Jan 5, 2024

jpzhangvincent commented Mar 6, 2024

ccurme commented Jul 8, 2024

jpzhangvincent commented May 21, 2023 •

edited

Loading

vercel bot commented Jun 27, 2023 •

edited

Loading