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

fix azure by removing model checking #495

Merged
merged 1 commit into from
Jan 18, 2024
Merged

fix azure by removing model checking #495

merged 1 commit into from
Jan 18, 2024

Conversation

PCSwingle
Copy link
Member

Fixes #489

Pull Request Checklist

  • Documentation has been updated, or this change doesn't require that

@biobootloader
Copy link
Member

If the user sets the model name as gpt-35-turbo (instead of gpt-3.5-turbo, like in the issue that prompted this), then will we not have cost estimates for them?

@PCSwingle
Copy link
Member Author

If the user sets the model name as gpt-35-turbo (instead of gpt-3.5-turbo, like in the issue that prompted this), then will we not have cost estimates for them?

We won't but there's no way to do that; you name the azure model when you set it up, so it could be anything.

@jakethekoenig
Copy link
Member

Two things that might make sense to do (not as part of this pr):

  1. Let the user set the costs in the config. Sort of like how they can set maximum_context.
  2. Guess what prices we should use by embedding their model name and seeing what model it's closest to.

@biobootloader
Copy link
Member

how about let the user set a base model to use for cost estimate as well as the model name to call?

so there'd be an extra variable like azure_deployment_true_model_name or something. If they don't set it then they'd get a warning when using azure that we don't know model limits or costs unless they set that

@PCSwingle
Copy link
Member Author

I think this adds too much complexity for no good reason; if anything, we would want to add a config option to set the input and output token prices (but I also don't think we should do that since it's too complicated for so little gain).

@PCSwingle PCSwingle merged commit 62ba2a6 into main Jan 18, 2024
16 checks passed
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.

Error when deployment_name is different from model name in Azure
3 participants