Implemented GroqProvider

dphiggs01 commented 1 month ago

Hi @assafelovic

This pull request implements the GroqProvider class, following the coding patterns defined in OpenAIProvider.

GroqProvider enables easy access to the below LLMs:

Llama3-70b-8192
Llama3-8b-8192
Gemma-7b-it
Mixtral-8x7b-32768

I have tested the changes using the local client and the examples/sample_report.py program with simple queries and can confirm positive results.

The Llama3 models work well but are constrained by the current 8K token window.

Gemma and Mixtral also run to completion with satisfactory results on simple queries even though they refuse to generate valid JSON to establish the "agent_role_prompt." (Note: I tried minor modifications to the auto_agent_instructions prompt but did not get positive results.)

Overall, the models work but are not close to the results generated by gpt-4o. However, they will continue to improve even in the short term. Given a larger context window and prompts customized to the nuances of the LLMs, I suspect Llama3 could be competitive with GPT-4 soon.

Thanks, Dan

gschmutz commented 1 month ago

Hi Dan (@dphiggs01) took your PR as an inspiration to implement support for Ollama tonight. In testing the changes, I realized that there is another section in the code where the llm_provider is checked in llm.py in the construct_subtopics function:

        if config.llm_provider == "openai":
            model = ChatOpenAI(model=config.smart_llm_model)
        elif config.llm_provider == "azureopenai":
            from langchain_openai import AzureChatOpenAI
            model = AzureChatOpenAI(model=config.smart_llm_model)
        else:
            return []

Think you should add groq here as well.

dphiggs01 commented 1 month ago

Think you should add groq here as well.

Hi @gschmutz ,

Thanks for the review and awesome you are going to implement Ollama!!

I'm not at the computer, but I will check when I get home. I think the better move might be to remove that code and call def get_provider(llm_provider): from this location, so we are not duplicating code. It does not smell right :) Thoughts?

Thanks, Dan

dphiggs01 commented 1 month ago

HI @gschmutz,

Thanks again for pointing out this oversite.

After reviewing the code, I thought a different approach might improve reliability and maintainability down the road.

As new Provider Classes are added, no changes are required here.
Also, since ProvidercClass manages the construction of the Chat[ObjectType], a higher level of control can be applied (e.g., enforcing the setting of the max_tokens and temperature as opposed to accepting the defaults). This is a good use of encapsulation.

temperature = config.temperature
ProviderClass = get_provider(config.llm_provider)
provider = ProviderClass(model=config.smart_llm_model,
                         temperature=temperature,
                         max_tokens=config.smart_token_limit)
model = provider.llm

As always, any thoughts or comments on improvements are welcome!

Thanks, -Dan

assafelovic commented 1 month ago

@dphiggs01 thank you very much for the effort. Big fan of Groq and definitely valuable for long research reports. I've added a new documentation page specifically for this here: https://github.com/assafelovic/gpt-researcher/blob/master/docs/docs/gpt-researcher/llms.md Please merge it to this branch and feel free to update it with all information to make it run. Thank you!

gschmutz commented 1 month ago

HI @gschmutz,

Thanks again for pointing out this oversite.

After reviewing the code, I thought a different approach might improve reliability and maintainability down the road.

* As new Provider Classes are added, no changes are required here.

* Also, since ProvidercClass manages the construction of the Chat[ObjectType], a higher level of control can be applied (e.g., enforcing the setting of the `max_tokens` and `temperature` as opposed to accepting the defaults). _This is a good use of encapsulation._

temperature = config.temperature
ProviderClass = get_provider(config.llm_provider)
provider = ProviderClass(model=config.smart_llm_model,
                         temperature=temperature,
                         max_tokens=config.smart_token_limit)
model = provider.llm

As always, any thoughts or comments on improvements are welcome!

Thanks, -Dan

I saw that code duplication as well and it's definitely more elegant like that!

Thanks Guido

gschmutz commented 1 month ago

@dphiggs01 thank you very much for the effort. Big fan of Groq and definitely valuable for long research reports. I've added a new documentation page specifically for this here: https://github.com/assafelovic/gpt-researcher/blob/master/docs/docs/gpt-researcher/llms.md Please merge it to this branch and feel free to update it with all information to make it run. Thank you!

Hi @dphiggs01 you find the way I have documented the configuration settings for Ollama in my PR. Think it would be nice to use the same "style". Have kept it simple and compact...

dphiggs01 commented 1 month ago

Hi @dphiggs01 you find the way I have documented the configuration settings for Ollama in my PR. Think it would be nice to use the same "style". Have kept it simple and compact...

Hi @gschmutz, Thanks for doing the initial pass on the documents. I followed your style, but since you need to sign up for the service, I had to add some additional detail.

I meant to merge the commit here, but apparently, I clicked the wrong button and opened a new PR. Still, there should be no conflicts.

Thanks!

assafelovic commented 1 month ago

@dphiggs01 can you confirm this is tested and ready to merge?

assafelovic / gpt-researcher

Implemented GroqProvider #526