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 GitHub Models support #113

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gs-101
Copy link

@gs-101 gs-101 commented Nov 26, 2024

Github Models

Try, test, and deploy from a wide range of model types, sizes, and specializations.

Some points I'd like to address here:

  1. I didn't edit the NEWS file because I'm not sure what version this commit would end up in (looking at your versioning scheme, it seems kind of like Conventional Commits/Angular Commits, so I'd guess this commit would fit a 0.19.0 release). I can rebase this commit to fit any requests you make.
  2. I see that you attributed copyright to the FSF, as this is package is available in GNU Elpa. I previously contributed to Emacs core, but I'm still waiting for a response on my copyright assignment.
  3. This is a draft PR as I couldn't get it to actually work with how it's currently laid out (testing with ellama). I tried to follow what you wrote in llm-azure. Thi is the error I'm getting:

llm-request-plz-sync-raw-output: plz: Curl error: "Curl error", #s(plz-error (6 . "Couldn't resolve host. The given remote host was not resolved.") nil nil)

With your experience I think you could easily spot the issue here, but I'll take a look at the other providers to see if I can fix it myself.

My gptel configuration, which works with this provider:

(use-package gptel-openai
  :after gptel
  :config
  (gptel-make-openai "Github Models"
    :host "models.inference.ai.azure.com"
    :endpoint "/chat/completions"
    :stream t
    :key #'gptel-api-key
    :models '(gpt-4o gpt-4o-mini meta-llama-3.1-405b-instruct llama-3.2-90B-vision-instruct)))

gptel-api-key gets the key from .authinfo.gpg, formated as:

machine models.inference.ai.azure.com login apiKey password GITHUB_TOKEN

@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 10:54 — with GitHub Actions Error
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 10:54 — with GitHub Actions Failure
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 10:54 — with GitHub Actions Error
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 10:54 — with GitHub Actions Failure
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 10:54 — with GitHub Actions Failure
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 10:54 — with GitHub Actions Error
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 10:54 — with GitHub Actions Error
@gs-101 gs-101 force-pushed the models/github-models branch from 2af31f7 to 95029aa Compare November 26, 2024 14:57
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 14:57 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 14:58 — with GitHub Actions Inactive
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 14:58 — with GitHub Actions Failure
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 14:58 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 14:58 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 14:58 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 14:58 — with GitHub Actions Inactive
@gs-101
Copy link
Author

gs-101 commented Nov 26, 2024

Some changes to see what CI tells me.

EDIT: Nice, but Ellama still tells me that the request was timed out.

Here's how I configured it:

(use-package ellama
  :bind ("C-z ," . ellama-transient-main-menu)
  :demand t
  :ensure t
  :init
  (setopt ellama-language "English")
  (require 'llm-github)
  (setopt ellama-provider
          (make-llm-github
           :chat-model "meta-llama-3.1-405b-instruct"
           :key "GITHUB_TOKEN"
           :embedding-model "text-embedding-3-large"
           :url "models.inference.ai.azure.com/"))
  (setopt ellama-summarization-provider
          (make-llm-github
           :chat-model "meta-llama-3.1-405b-instruct"
           :key "GITHUB_TOKEN"
           :embedding-model "text-embedding-3-large"
           :url "models.inference.ai.azure.com/"))
  (setopt ellama-coding-provider
          (make-llm-github
           :chat-model "meta-llama-3.1-405b-instruct"
           :key "GITHUB_TOKEN"
           :embedding-model "text-embedding-3-large"
           :url "models.inference.ai.azure.com/"))
  (setopt ellama-naming-provider
          (make-llm-github
           :chat-model "meta-llama-3.1-405b-instruct"
           :key "GITHUB_TOKEN"
           :embedding-model "text-embedding-3-large"
           :url "models.inference.ai.azure.com/"))
  (setopt ellama-naming-scheme 'ellama-generate-name-by-llm)
  (setopt ellama-translation-provider
          (make-llm-github
           :chat-model "meta-llama-3.1-405b-instruct"
           :key "GITHUB_TOKEN"
           :embedding-model "text-embedding-3-large"
           :url "models.inference.ai.azure.com/")))

I still need to figure out how to set :url inside of the original file, as it doens't change. To solve the time outs I can probably just make a CURL request and see how it is sent again.

llm-github.el Outdated Show resolved Hide resolved
llm-github.el Outdated Show resolved Hide resolved
@ahyatt
Copy link
Owner

ahyatt commented Nov 26, 2024

Thanks for this PR! Let me know when you get your FSF paperwork finished, and I can merge this in. If you are having any issues with it, please inform me so I can ask the maintainers.

@gs-101 gs-101 force-pushed the models/github-models branch from 95029aa to 1e3a468 Compare November 26, 2024 16:52
@gs-101 gs-101 had a problem deploying to Continuous Integration November 26, 2024 16:52 — with GitHub Actions Failure
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 16:52 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 16:52 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 16:52 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 16:52 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 16:52 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration November 26, 2024 16:52 — with GitHub Actions Inactive
@gs-101
Copy link
Author

gs-101 commented Nov 26, 2024

[...] Let me know when you get your FSF paperwork finished [...]

On this, does it take long? I sent my request to [email protected] last saturday (2024-11-23).

@ahyatt
Copy link
Owner

ahyatt commented Nov 26, 2024

[...] Let me know when you get your FSF paperwork finished [...]

On this, does it take long? I sent my request to [email protected] last saturday (2024-11-23).

Have they replied? It usually is fast for them to reply, but from the time for them to send and receive your signed paperwork can take weeks.

@gs-101
Copy link
Author

gs-101 commented Nov 26, 2024

Have they replied? [...]

No, not yet. But I sent it during the weekend so I think it's fair of them to take their time. I'll wait and if there's no response this saturday I think I'll try to contact them again.

@gs-101
Copy link
Author

gs-101 commented Dec 17, 2024

Hey, just getting back on this. I just got my paper signed!

If it applies beyond Emacs core source code (I need to check on ELPA for this), then this PR can be merged.

@gs-101 gs-101 marked this pull request as ready for review December 17, 2024 23:12
(require 'llm-openai)
(require 'cl-lib)

(cl-defstruct (llm-github (:include llm-openai-compatible (url "https://models.inference.ai.azure.com"))))
Copy link
Owner

Choose a reason for hiding this comment

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

Interesting that this is an Azure URL. Can or should we base it off of the azure provider instead? Perhaps it could just be a Azure provider with a different default value for the URL.

Copy link
Author

Choose a reason for hiding this comment

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

Interesting that this is an Azure URL. Can or should we base it off of the azure provider instead? Perhaps it could just be a Azure provider with a different default value for the URL.

It could be, as it also supports the Azure SDK:

07:38 AM 18-12-2024

Python Example on the page:

import os
from azure.ai.inference import ChatCompletionsClient
from azure.ai.inference.models import SystemMessage, UserMessage
from azure.core.credentials import AzureKeyCredential

endpoint = "https://models.inference.ai.azure.com"
model_name = "Meta-Llama-3.1-405B-Instruct"
token = os.environ["GITHUB_TOKEN"]

client = ChatCompletionsClient(
    endpoint=endpoint,
    credential=AzureKeyCredential(token),
)

response = client.complete(
    messages=[
        SystemMessage(content="You are a helpful assistant."),
        UserMessage(content="What is the capital of France?"),
    ],
    temperature=1.0,
    top_p=1.0,
    max_tokens=1000,
    model=model_name
)

print(response.choices[0].message.content)

For basing it off (as in, making a new file like I did), I don't think it would be necessary, as it already works (the page on the screenshot only has the Azure example, but I was able to make a request with the current configuration).

But, if we were to expand the Azure provider to support Github Models, that would require setting the chat and embedding URL as arguments, or add a conditional to change the formatting URL.

Example with chat URL:

(cl-defmethod llm-provider-chat-url ((provider llm-azure))
-  (format "%s/openai/deployments/%s/chat/completions?api-version=2024-08-01-preview"
-          (llm-azure-url provider)
-          (llm-azure-chat-model provider)))
(cl-defmethod llm-provider-chat-url ((provider llm-azure))
+  (if (llm-azure-github t)
+      (format "%s/chat/completions"
+              (llm-azure-url provider))
+    (format "%s/openai/deployments/%s/chat/completions?api-version=2024-08-01-preview"
+            (llm-azure-url provider)
+            (llm-azure-chat-model provider))))

Copy link
Owner

Choose a reason for hiding this comment

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

You shouldn't need to do the if, though, you could have your struct include the llm-azure struct, and then you could keep your code as normal.

(cl-defmethod llm-provider-chat-url ((provider llm-github))
  (format "%s/chat/completions" (llm-azure-url provider))))

Copy link
Author

@gs-101 gs-101 Dec 18, 2024

Choose a reason for hiding this comment

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

You shouldn't need to do the if, though, you could have your struct include the llm-azure struct, and then you could keep your code as normal.

Thanks, sorry about that, I don't know much about Common Lisp.

I tried both of these:

-(require 'llm-openai)
+(require 'llm-azure)

-(cl-defstruct (llm-github (:include llm-openai-compatible (url "https://models.inference.ai.azure.com"))))
+(cl-defstruct (llm-github (:include llm-azure (url "https://models.inference.ai.azure.com"))))
-(require 'llm-openai)
+(require 'llm-azure)

-(cl-defstruct (llm-github (:include llm-openai-compatible (url "https://models.inference.ai.azure.com"))))
+(cl-defstruct (llm-github (:include llm-azure (url "https://models.inference.ai.azure.com"))))

 (cl-defmethod llm-provider-chat-url ((provider llm-github))
   (format "%s/chat/completions"
-          (llm-github-url provider)))
+          (llm-azure-url provider)))

 (cl-defmethod llm-provider-embedding-url ((provider llm-github) &optional _)
   (format "%s/embeddings/"
-          (llm-github-url provider)))
+          (llm-azure-url provider)))

But my requests timed out.

README.org Show resolved Hide resolved
@ahyatt
Copy link
Owner

ahyatt commented Dec 18, 2024

Hey, just getting back on this. I just got my paper signed!

If it applies beyond Emacs core source code (I need to check on ELPA for this), then this PR can be merged.

This is fantastic, thanks for going through that! With that in mind, I've finally looked at your change and left a few comments.

@gs-101 gs-101 force-pushed the models/github-models branch from 1e3a468 to bea8036 Compare December 18, 2024 17:07
@gs-101 gs-101 temporarily deployed to Continuous Integration December 18, 2024 17:08 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration December 18, 2024 17:08 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration December 18, 2024 17:08 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration December 18, 2024 17:08 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration December 18, 2024 17:08 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration December 18, 2024 17:08 — with GitHub Actions Inactive
@gs-101 gs-101 had a problem deploying to Continuous Integration December 18, 2024 17:08 — with GitHub Actions Failure
@gs-101 gs-101 force-pushed the models/github-models branch from bea8036 to dc4073a Compare January 12, 2025 00:34
@gs-101 gs-101 temporarily deployed to Continuous Integration January 12, 2025 00:34 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration January 12, 2025 00:34 — with GitHub Actions Inactive
@gs-101 gs-101 had a problem deploying to Continuous Integration January 12, 2025 00:34 — with GitHub Actions Failure
@gs-101 gs-101 temporarily deployed to Continuous Integration January 12, 2025 00:34 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration January 12, 2025 00:34 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration January 12, 2025 00:34 — with GitHub Actions Inactive
@gs-101 gs-101 temporarily deployed to Continuous Integration January 12, 2025 00:36 — with GitHub Actions Inactive
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.

2 participants