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

Added support for local LLM models via Ollama #30

Open
wants to merge 267 commits into
base: main
Choose a base branch
from

Conversation

devparanjay
Copy link

@devparanjay devparanjay commented Apr 8, 2024

Summary

Added support for Ollama to be able to use local LLM models.
FOSS LLMs a necessity for the future of Open Source Artificial Intelligence. This is an attempt to add that functionality to Sirji (although it was done in Windows and Sirji works on Mac for now).
A .env file is used to set the OpenAI API Key, Ollama model, and to set whether Ollama is used.

What are the specific steps to test this change?

  1. Set the environment variables in .env.
  2. Run Sirji like usual and it ideally should work like it is supposed to.

What kind of change does this PR introduce?

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Build-related changes
  • Other

Make sure the PR fulfills these requirements:

  • It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
  • The changelog is updated
  • Related documentation has been updated
  • Related tests have been updated

Other information:
I wasn't able to test this since I use a Windows machine.
I use Ruff for formatting in my IDE - explaining the format changes in code.
The important change-related files are the new .env, sirji/config/model.py, everywhere the OpenAI API key was being called, and wherever OpenAI completions was used and model was set.
(Unfortunately, I did not read the PR guideline earlier so skipped the "feature request issue" part; apologies for not raising it before the PR.)

kedarchandrayan and others added 30 commits April 2, 2024 18:42
Merging PyPI packages to vs code extension branch
Merging the current state of vs code UI to vs code extension
main.py Outdated
@@ -258,6 +270,7 @@ def perform_non_gui_tasks():

if __name__ == "__main__":
empty_workspace()
load_dotenv()
Copy link
Author

Choose a reason for hiding this comment

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

loaded dotenv

requirements.txt Outdated
Copy link
Author

Choose a reason for hiding this comment

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

Added python-dotenv

@kedarchandrayan
Copy link
Member

Hi @devparanjay, this is Kedar, a co-maintainer of Sirji. Thanks for your contribution!

I've reviewed your PR and appreciate the additional options for inference you've introduced.

However, we recently transitioned Sirji into a VS Code Extension, resulting in significant changes to the main branch, including conflicts with your PR.

The inference code now resides in a PyPI package within the agents folder of our monorepo. I suggest reviewing this folder before moving forward. If you have questions, feel free to ask.

Hello @devparanjay - please refer to the above-quoted message. I just now saw that you have committed to main.py which is no longer in the latest main branch.

As your branch has become outdated, I suggest starting afresh and making only the necessary changes. As mentioned by me earlier, the code has moved to agents folder of our monorepo. If you have questions, feel free to ask.

@devparanjay
Copy link
Author

Hi @devparanjay, this is Kedar, a co-maintainer of Sirji. Thanks for your contribution!
I've reviewed your PR and appreciate the additional options for inference you've introduced.
However, we recently transitioned Sirji into a VS Code Extension, resulting in significant changes to the main branch, including conflicts with your PR.
The inference code now resides in a PyPI package within the agents folder of our monorepo. I suggest reviewing this folder before moving forward. If you have questions, feel free to ask.

Hello @devparanjay - please refer to the above-quoted message. I just now saw that you have committed to main.py which is no longer in the latest main branch.

As your branch has become outdated, I suggest starting afresh and making only the necessary changes. As mentioned by me earlier, the code has moved to agents folder of our monorepo. If you have questions, feel free to ask.

Hey, yes, sorry - the comments were for my benefit since I did this a while back and was just checking what changes I did where. I'll try to add this to the new code today or in the next couple of days. It does look like that starting from scratch would be good.

@devparanjay
Copy link
Author

devparanjay commented Apr 24, 2024

Hello @kedarchandrayan,

Sirji should now ideally work with Ollama. I've updated the README and added relevant settings in the Environment Variable section in views/chat.html and views/chat.js for the extension.
I've not previously worked with VSCode extensions so I'm not sure how it handles setting environment variables behind the code, but if no other file had to be modified to set environment variables other than chat.js, then I think it should be fine.

Please review.

Git seems to be recounting all of yours' commits again for some reason (probably because of forked branch merges from my repo), but here's a list of relevant files for your convenience -

  • agents/sirji_agents/llm/model_providers/factory.py
  • agents/sirji_agents/llm/model_providers/ollama.py
  • agents/sirji_agents/researcher/embeddings/openai_assistant.py
  • agents/sirji_agents/researcher/inferer/openai_assistant.py
  • sirji/vscode-extension/src/views/chat/chat.html
  • sirji/vscode-extension/src/views/chat/chat.js

@kedarchandrayan
Copy link
Member

Hello @devparanjay,

Currently, the total number of file changes is showing as 214, which is not possible. As you mentioned, it seems to be an issue that Git is not identifying correctly.

For ease of review, I would request that you fork again and make only the necessary changes to the specific files. This will help us avoid committing any unintentional changes. Please create a fresh PR; we can close the current PR later by referencing it in the new PR.

@devparanjay
Copy link
Author

Hello @kedarchandrayan,

I've opened a new PR #110, please close this PR if everything looks fine for review in the new PR.

I had forced a merge of two branches in my forked repo with -s ours to deal with the major change in project structure. That was making git re-count all the changes in git history that were made to upstream's main branch, resulting in this anomaly. I made a clean branch from upstream/main and incorporated my changes there for the new PR and things should be more normal now. Apologies for the delay.

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.

8 participants