add Google assistants #301
Conversation
| @@ -18,7 +18,7 @@ class ApiWrapper(param.Parameterized): | |||
| auth_token = param.String(default=None) | |||
|
|
|||
| def __init__(self, api_url, **params): | |||
| self.client = httpx.AsyncClient(base_url=api_url) | |||
| self.client = httpx.AsyncClient(base_url=api_url, timeout=60) | |||
There was a problem hiding this comment.
Even when streaming, the Google assistants return really large chunks and thus easily go over the default timeout. The new timeout is in line with what we use for our builtin assistants as well:
ragna/ragna/assistants/_api.py
Lines 19 to 22 in 1b53e62
docs/references/faq.md
Outdated
|
|
||
| ### [Google](https://ai.google.dev/) | ||
|
|
||
| 1. ADDME |
There was a problem hiding this comment.
@nenb Could you fill out this similar to what we did for the others?
|
Why did you choose |
|
Two reasons:
|
|
|
||
| async def anext(ait: AsyncIterator[T]) -> T: | ||
| return await ait.__anext__() | ||
| sentinel = object() |
There was a problem hiding this comment.
This is a slight refactor as I needed the default return value in case of exhaustion.
|
I've opted to go with |
| ], | ||
| # https://ai.google.dev/tutorials/rest_quickstart#configuration | ||
| "generationConfig": { | ||
| "temperature": 0.0, |
There was a problem hiding this comment.
If we are going to hard-code this then I would suggest a higher-value, as this is what most users will require.
There was a problem hiding this comment.
It is the other way around: we want to hardcode 0.0 here, because that means determinism. If we learned one thing from trying to bring RAG to businesses is that they want to get exactly the same answer if they ask the same question twice. Of course we can't guarantee it since we don't control the model, but we can do our best to at least avoid sampling during generation.
This is the same for all other assistants that we currently have
ragna/ragna/assistants/_anthropic.py
Line 52 in 1b53e62
ragna/ragna/assistants/_mosaicml.py
Line 43 in 1b53e62
ragna/ragna/assistants/_openai.py
Line 58 in 1b53e62
| self._ait = ait | ||
|
|
||
| async def read(self, n: int) -> bytes: | ||
| if n == 0: |
There was a problem hiding this comment.
Nitpick: could you add some documentation here on why the n arg is required/used? I get that ijson expects a file-like object, but does this also imply the n arg, or is it something specific to ijson.
ragna/_compat.py
Outdated
| default: T = sentinel, # type: ignore[assignment] | ||
| ) -> Awaitable[T]: | ||
| if default is sentinel: | ||
| return ait.__anext__() |
There was a problem hiding this comment.
I don't fully grok this. Do we not need to await this? And in what situation will this arise?
There was a problem hiding this comment.
I don't fully grok this. Do we not need to
awaitthis?
We don't and we actually can't here. Note that anext is not an async def function. anext returns an awaitable, e.g.
async_iterator = ...
awaitable = anext(async_iterator)
result = await awaitableAnd in what situation will this arise?
This is the default case, i.e. no default value is set. Let me refactor this function to make it more clear to what is going on.
Co-authored-by: Nick Byrne <55434794+nenb@users.noreply.github.com>
This adds the Gemini Pro and Ultra models from Google to the builtin assistants. To achieve that I needed to do some minor refactoring. I'll explain it below.