Skip to content

Implement patched o1 & o1-mini model functionality #160

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

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

Conversation

Broken-Admin
Copy link

Issue to be resolved
Currently the o1 and o1-mini models do not support the system role.
See issue #159

Solution
This patch resolves the issue by generating custom system messages for those models.

Possible issues or concerns
OpenAI is expected to potentially implement the "system" role in future updates.

* OpenAI intends to implement the "system" role in future updates
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. inject different "user" role instead of "system" if the model is "o1-mini" (o1 does not reproduce the issue), with minimal code and logic changes.
  2. ask only one time if the model is "o1-mini" and then make all changes necessary to the system, messages.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Removed duplicated prompts and simply change the config.py prompts role when o1-mini is detected.
  2. Moved the logic the the cli method that is run each call. Model type is only checked once in this method and necessary prompt role is updated.

* Modify logic to only change role of messages when necessary rather than duplicate
* Change logic to only o1-mini model
* Only check the model type once
* Modify the necessary prompts to run the command with the o1-model
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