-
Notifications
You must be signed in to change notification settings - Fork 22
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 gemma2 models of 2 sizes #352
Conversation
@@ -194,35 +194,6 @@ func (apiServer *HelixAPIServer) createSession(res http.ResponseWriter, req *htt | |||
return nil, err | |||
} | |||
|
|||
helixModel := req.FormValue("helixModel") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure this doesn't break the frontend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah I see the refactor now, sorry, ignore this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is an odd one - we don't even use the POST /api/v1/sessions
route any more - we post to the /chat
and /learn
endpoints now
So - this old code is basically dead - and yet I refactor the function that filters the model name because it stinks that it's in two places but I'm not quite mentally ready to be deleting big chunks of code in this PR
So we get this strange middle ground that actually doesn't do anything 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK this is not true - the preview for tools and apps both still use the old POST /api/v1/sessions
route - we should tidy these both up to use the new /chat
route
frontend/src/types.ts
Outdated
export const MODEL_NAME_MISTRAL: IModelName = 'mistralai/Mistral-7B-Instruct-v0.1' | ||
export const MODEL_NAME_SDXL: IModelName = 'stabilityai/stable-diffusion-xl-base-1.0' | ||
export const MODEL_NAME_OLLAMA_MISTRAL: IModelName = 'mistral:7b-instruct' | ||
export const MODEL_NAME_OLLAMA_LLAMA3_8B: IModelName = 'llama3:instruct' | ||
export const MODEL_NAME_OLLAMA_LLAMA3_70B: IModelName = 'llama3:70b' | ||
export const MODEL_NAME_OLLAMA_MIXTRAL: IModelName = 'mixtral:instruct' | ||
export const MODEL_NAME_OLLAMA_PHI3: IModelName = 'phi3:instruct' | ||
export const MODEL_NAME_GEMMA2_9B: IModelName = 'gemma2:9b' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be ...OLLAMA_GEMMA2... for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome
This is NOT working yet |
add the gemma2 model in both sizes