-
Notifications
You must be signed in to change notification settings - Fork 634
FEAT: adding underlying_model for target identification #1234
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
base: main
Are you sure you want to change the base?
Conversation
|
great work Justin, I think this makes a lot of sense how the deployment name could be arbitrary but the actual type of deployment is useful esp for metrics! |
bashirpartovi
left a comment
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.
Looks great Justin
| If the underlying model is specified, either passed in during instantiation or via environment variable, | ||
| it is used as the model_name for the identifier. Otherwise, self._model_name (which is often the | ||
| deployment name in Azure) is used. |
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.
Suggestion:
Use backtick to highlight variables in your docstring. e.g. "If the underlying_model is ... "
Description
This draft adds
underlying_modelparam and environment variable which differs frommodel_namewhich is often actually the deployment name (i.e. Azure endpoints). Theunderlying_modelwill give better insight into the actual model used rather than the deployment and will be useful for identifying scoring configurations and their associated evaluation metrics (since deployment names can be custom made).underlying_modelcan be passed in explicitly as a param in the constructor or set via environment variable (e.g.OPENAI_CHAT_UNDERLYING_MODEL)UPDATE: We are not implementing method to get the underlying model name programmatically in this PR.
This also adds a methodget_underlying_model_asyncto get the underlying model name programmatically for certain OpenAI targets that return themodelin the response (by sending a very simple prompt, I couldn't get GETs to work...). (The reason why it is not done automatically upon get_identifier is because it seems like overkill to make the method async throughout the codebase. We also can't call it upon instantiation because async inits aren't a thing). So, calling this method is manual, and the preferred way would be through environment variable or passed value, which would flow into the identifier.Tests and Documentation
Unit tests added