-
Notifications
You must be signed in to change notification settings - Fork 547
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 generation params to inference engines #600
Conversation
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.
If you want to simplify the warning logic, we could implement a get_supported_params
method in the baseInferenceEngine that by default returns an empty set. This method is called by infer()
and will print warnings where relevant. Each derivative class could simply implement that method and get alerting as needed. Food for thought though, not completely necessary as we only have a few engines.
Additionally, we should update our unit tests to verify that the new parameters are passed appropriately
That's a great suggestion -- logged OPE-546 to address as a follow-up. Update this PR to include unit tests for the generation params |
Add Generation Parameters to Inference Engines
This PR introduces a set of missing generation parameters, and updates all the current inference engines to support them, or if not supported by the engine, logs a warning that the parameter will be ignored.
The following parameters are added: temperature, top_p, frequency_penalty, presence_penalty, stop sequences, logit_bias, and min_p.
Towards OPE-328
Changes
GenerationParams
class with new parametersUsage
Setting Generation Parameters
Using Parameters with an Inference Engine