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

New models, Google Gemini, some adjustements #25

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zaizou
Copy link

@zaizou zaizou commented Feb 20, 2024

In this update,

  • the Google Gemini Pro API model was integrated. It's important to note that Gemini isn't currently available in European countries. If you want to use it, you'll need to use a VPN.
  • Replaced Ploty for Plotly in the demo.py file to offer a more user-friendly graphical interface.
  • Added two draft models based on the Mistral API, specifically customized for finance.
  • made demo.py lightweight for faster generation. I chose to prioritize direct generation over working on hyperparameters.

Please note that the NLL calculation is not yet implemented for google, also for the past mistral model.

@zaizou
Copy link
Author

zaizou commented Feb 29, 2024

Are you we okey ?

@ngruver
Copy link
Owner

ngruver commented Mar 1, 2024

Hello,

I think I will need to update the readme (beyond what is proposed in the PR) to make it more clear now that there are many model options. I can take a look this weekend.

@zaizou
Copy link
Author

zaizou commented Mar 2, 2024

Okey,
Maybe also working on the architecture (OOP), so when a company modify or add a model (API) (Mistral large, and gemini ultra for instance) we don't have to code. It will be interesting that we discuss about the feature evolutions if it will some. Maybe some integration with other research and thesis.
Let me know if you're open for a collaboration I have some ideas 👍

@ngruver
Copy link
Owner

ngruver commented Mar 2, 2024

I'm reading through your PR and I noticed there might be an issue in the last set of changes. In the code for the mistral API, it looks like you are using the OpenAI tokenizers: https://github.com/ngruver/llmtime/blob/main/models/mistral_api.py#L33. Did you confirm that this makes? It probably makes more sense to use the huggingface mistral tokenizers, unless it is known that their API models use a different tokenizer.

@ngruver
Copy link
Owner

ngruver commented Mar 2, 2024

Other issues/comments:

  • What is the difference between "mistral-api" models and "mistral-api-stocks" models? Is it just a different prompt applied to the same models? There might be a simpler way to implement this, by adding a "sys_message" argument for example.
  • .vscode files should be removed from the PR
  • finbert_utils.py should also be removed, as I'm not seeing how it is used

@zaizou
Copy link
Author

zaizou commented Mar 8, 2024

Hello,
Of course all of your remark are good
I wanted to pass the system message as a parameter but I didn't have much time, I thought also about making things OOP
To two other points will be done soon

@zaizou
Copy link
Author

zaizou commented Mar 8, 2024

Also for Mistral tokenizer ok
Tell me if you wanna do it yourself

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