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

Add base API URL field for Ollama and OpenAI embedding models #1136

Merged
merged 7 commits into from
Dec 9, 2024

Conversation

srdas
Copy link
Collaborator

@srdas srdas commented Dec 4, 2024

Description

Jupyter AI currently allows the user to call a model at a URL (location) different from the default one by specifying a selected Base API URL. This can be done for Ollama, OpenAI provider models. However, for these providers, there is no way to change the API URL for embedding models when using the /learn command in RAG mode. This PR adds an extra field to make this feasible.

Testing instructions

Testing as follows for Ollama:
[1] Start the Ollama system from port 11435 instead 11434 (the default):
OLLAMA_HOST=127.0.0.1:11435 ollama serve
[2] Set the Base API URL:
set base api url
[3] Check that the new API URL works for completions and /learn:

@srdas srdas added the enhancement New feature or request label Dec 4, 2024
srdas and others added 2 commits December 5, 2024 15:27
Jupyter AI currently allows the user to call a model at a URL (location) different from the default one by specifying a selected Base API URL. This can be done for Ollama, OpenAI provider models. However, for these providers, there is no way to change the API URL for embedding models when using the `/learn` command in RAG mode. This PR adds an extra field to make this feasible.

Tested as follows for Ollama:
[1] Start the Ollama system from port 11435 instead 11434 (the default):
`OLLAMA_HOST=127.0.0.1:11435 ollama serve`
[2] Set the Base API URL:

[3] Check that the new API URL works:
@dlqqq
Copy link
Member

dlqqq commented Dec 9, 2024

I noticed a bug on a24b436 on Friday but didn't have time to document it. If you change the OpenAI embeddings base URL to an empty value and save, all subsequent connections fail. I did some debugging and found that the OpenAIEmbeddings provider class fails if you pass it openai_api_base="", because it treats the empty string as the API base instead of ignoring it.

I'm pushing a change that alters the ConfigManager to exclude empty string fields from the keyword arguments it provides. This fixes the bug for me locally. Most providers already ignore optional keyword arguments that are set to an empty string, so this shouldn't cause any change for users.

@dlqqq dlqqq changed the title Base API URL added for embedding models Add base API URL field for Ollama and OpenAI embedding models Dec 9, 2024
@dlqqq dlqqq marked this pull request as ready for review December 9, 2024 19:52
Copy link
Collaborator Author

@srdas srdas left a comment

Choose a reason for hiding this comment

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

  1. Tested /learn and /ask with haiku-3.5 and titan-embed-v1. ✅ (No Base API URL)
  2. Tested /learn and /ask with ollama-llama3.2 and ollama-mxbai-embed-large. ✅ (No Base API URL) -- all next tests with Ollama (to test changes in ollama.py ).
  3. Tested again with Base API URL = 11434 (the default explicitly). ✅
  4. Tested again after clearing out the Base API URL, still works ✅
  5. Restarted Ollama with port=12345. Added this to the Base API URL and it all works as expected. ✅
  6. Removed the custom Base API URL (blank field) and the /learn and /ask commands now fail, as they should, because Ollama is still running on the custom port. :gr-checkmark:
  7. Leaving custom fields blank, restarted Ollama to return to default API URL and everything works as expected. ✅
  8. With OpenAI embeddings, left the Base API URL blank (it works ✅), then added the URL (it works ✅) and then deleted the URL (and it still works ✅, confirms the change in config_manager.py is implemented).

Code looks good as well.

@dlqqq
Copy link
Member

dlqqq commented Dec 9, 2024

Kicking CI since the RTD workflow has stalled.

@dlqqq dlqqq closed this Dec 9, 2024
@dlqqq dlqqq reopened this Dec 9, 2024
@dlqqq dlqqq merged commit 1cbd853 into jupyterlab:main Dec 9, 2024
20 checks passed
@dlqqq
Copy link
Member

dlqqq commented Dec 9, 2024

@meeseeksdev please backport to v3-dev

meeseeksmachine pushed a commit to meeseeksmachine/jupyter-ai that referenced this pull request Dec 9, 2024
dlqqq pushed a commit that referenced this pull request Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants