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.

Sign up for GitHub

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

Jump to bottom

Feature/azure openai support #176

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

Feature/azure openai support #176

wants to merge 10 commits into from

Conversation

Alex-Wenner-FHR
Copy link

@Alex-Wenner-FHR Alex-Wenner-FHR commented Dec 18, 2024 •
edited
Loading

Description

Adds support for Azure OpenAI

How Has This Been Tested?

I populated the keys locally and ran a training job and it had succeeded.

  • Didn't see applicable unit tests, could have missed them
  • Unit tests (pytest tests/)
  • Integration tests (if applicable)

Configuration Changes

Added details in the README of necessary env variables and added those into the configure_llm.sh

  • No config changes
  • Config changes (please describe):

Type of Change

  • [ x ] New feature

FYI looked like my Ruff formatter did some formatting changes... happy to undo if they do not meet standards. Looking for feedback, thanks!

Screenshot 2024-12-18 at 9 18 59 AM

Closes #171

@Alex-Wenner-FHR
Copy link
Author

@FANGAreNotGnu - looks like I am unable to add you or any other team members as reviewers. Also, I suppose I do not have necessary permissions to successfully execute actions. Let me know what I should do here. Thanks!

@FANGAreNotGnu FANGAreNotGnu self-requested a review December 26, 2024 21:19
@FANGAreNotGnu
Copy link
Contributor

Hi Alex, thanks for your contribution! Could you run these formatting commands on the code:

black src/
ruff --fix src/
isort src/

I'll review your PR in the meantime. Thanks!

FANGAreNotGnu
FANGAreNotGnu reviewed Dec 26, 2024
View reviewed changes
Copy link
Contributor

@FANGAreNotGnu FANGAreNotGnu left a comment

Choose a reason for hiding this comment

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

I've added some review comments. @tonyhoo @boranhan - we'll likely need an Azure API key for testing this.

@Alex-Wenner-FHR
Copy link
Author

@FANGAreNotGnu - addressing the first and the third comment! I will leave the second alone and let you address that as mentioned. Thanks!

@FANGAreNotGnu
Copy link
Contributor

I've updated the configuration. We can approve once we've tested it with an Azure API key. @boranhan @tonyhoo @AnirudhDagar

@FANGAreNotGnu
Copy link
Contributor

Status Update: Initial Azure OpenAI quota increase request was declined due to insufficient capacity. Second request has been submitted and is pending review.

@FANGAreNotGnu
Copy link
Contributor

Update: I asked for GPT4 base model but was granted quota of GPT35-turbo in finetune mode. And even the finetune is disabled due to capacity issues. Third request has been submitted and is pending review. We could consider merge it and test later.

FANGAreNotGnu
FANGAreNotGnu approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@FANGAreNotGnu FANGAreNotGnu left a comment

Choose a reason for hiding this comment

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

LGTM and thanks for the contribution! The PR has not been tested on our end due to Azure's insufficient capacity, but we should consider merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FANGAreNotGnu FANGAreNotGnu FANGAreNotGnu approved these changes

@boranhan boranhan Awaiting requested review from boranhan

@AnirudhDagar AnirudhDagar Awaiting requested review from AnirudhDagar

@tonyhoo tonyhoo Awaiting requested review from tonyhoo

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Azure OpenAI support
2 participants
@Alex-Wenner-FHR @FANGAreNotGnu