Azure / azureml-examples

Official community-driven Azure Machine Learning examples, tested with GitHub Actions.
https://docs.microsoft.com/azure/machine-learning
MIT License
1.76k stars 1.43k forks source link

LiteLLM Example in mistral docs wrong #3024

Open ishaan-jaff opened 8 months ago

ishaan-jaff commented 8 months ago

Operating System

MacOS

Version Information

not relevant

Steps to reproduce

https://github.com/Azure/azureml-examples/blob/main/sdk/python/foundation-models/mistral/litellm.ipynb @santiagxf Thanks for showing an example with litellm, but the docs are wrong

Here's how to use litellm with mistral https://docs.litellm.ai/docs/providers/mistral

from litellm import completion
import os

os.environ['MISTRAL_API_KEY'] = ""
response = completion(
    model="mistral/mistral-tiny", 
    api_base="your-api-base",
    messages=[
       {"role": "user", "content": "hello from litellm"}
   ],
)
print(response)

Expected behavior

-

Actual behavior

-

Addition information

No response

santiagxf commented 8 months ago

@ishaan-jaff, the example is correct. You are sharing an example about how to use Mistral's inference platform, but this is Azure AI. Does it make sense?

ishaan-jaff commented 8 months ago

any reason why you could not use it like this ? @santiagxf ? (i'm the maintainer of litellm)

This looks a lot easier to me and it can go to to an Azure AI endpoint

from litellm import completion
import os

os.environ['MISTRAL_API_KEY'] = ""
response = completion(
    model="mistral/mistral-tiny", 
    api_base="your-api-base",
    messages=[
       {"role": "user", "content": "hello from litellm"}
   ],
)
print(response)
ishaan-jaff commented 8 months ago

Hi @santiagxf I just deployed on Azure AI studio and I was able to run inference with this code

If possible can we update the python notebook with the following code ? It uses the standard format on litellm docs: https://docs.litellm.ai/docs/providers/azure_ai

Happy to make a PR for this too

from litellm import completion
import os

response = completion(
    model="mistral/Mistral-large-dfgfj", 
    api_base="https://Mistral-large-dfgfj-serverless.eastus2.inference.ai.azure.com/v1",
    api_key = "JGbKodRcTp****"
    messages=[
       {"role": "user", "content": "hello from litellm"}
   ],
)
print(response)
santiagxf commented 8 months ago

We tried your example, but it looks the api_base is hardcorded somewhere. Hence you get an access denied error because the token is going to the wrong API. You will get this error (I got this by setting litellm.set_verbose=True).

POST Request Sent from LiteLLM:
curl -X POST \
https://api.mistral.ai/v1/ \
-d '{'model': 'Mistral-large-dfgfj', 'messages': [{'role': 'user', 'content': 'hello from litellm'}], 'extra_body': {}}'

Is this something you can fix? @ishaan-jaff

ishaan-jaff commented 8 months ago

Yes we fixed this today: https://github.com/BerriAI/litellm/pull/2216 @santiagxf

Thanks for raising this

santiagxf commented 8 months ago

I just had a look and notice that it requires v1 at the end of the URL. mistral-python, the official client, doesn't require it. I think we should fix that to work in the same way.

https://github.com/BerriAI/litellm/blob/e56cc26e18b83aa39617b7d2cf8ce3c1e3dfee87/litellm/utils.py#L4931

Also, I spotted this line:

https://github.com/BerriAI/litellm/blob/e56cc26e18b83aa39617b7d2cf8ce3c1e3dfee87/litellm/utils.py#L4932

Does it means api_key is being ignored if passed as argument?

ishaan-jaff commented 8 months ago

Does it means api_key is being ignored if passed as argument?

Nope, that line ensure we use MISTRAL_API_KEY from the env

think we should fix that to work in the same way.

Do you mean litellm the package should append a /v1 to Azure AI studio api_base if the user does not add it ?

ishaan-jaff commented 8 months ago

Does it means api_key is being ignored if passed as argument?

I think you're right about this actually, going to take a look at this

Summarizing next steps from litellm

Does this sound good @santiagxf ? Should I make a PR to this repo once this is fixed

santiagxf commented 8 months ago

It sounds good to me. The URL thing is not a big deal, but I think it would be nice to use the same approach the official client uses.

Thanks for taking care of this! Once the new version of the library is out we can update this doc. Feel free to use this issue. I will keep it opened so I can remember to do it.

hsleiman1 commented 8 months ago

Hi, Why do we need to mention the api_base and api_key, which are not needed for the other models? It would be great if they are also read from the env variables. Thank you!

ishaan-jaff commented 8 months ago

@hsleiman1 noted - will add this as an improvement too

ishaan-jaff commented 8 months ago

Tracking this here @hsleiman1 https://github.com/BerriAI/litellm/issues/2237

ishaan-jaff commented 8 months ago

Fixes are here : https://github.com/BerriAI/litellm/pull/2247 @hsleiman1 @santiagxf, will update once a new release is out

santiagxf commented 7 months ago

@ishaan-jaff we still see the same issues happening. I added the comment in the issue you created on your repo:

https://github.com/BerriAI/litellm/issues/2237#issuecomment-2053668601