Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

adding finetuned model setup#62838

Merged
sagtanih merged 10 commits into
mainfrom
hitesh/finetuned-fim-model-integration
May 22, 2024
Merged

adding finetuned model setup#62838
sagtanih merged 10 commits into
mainfrom
hitesh/finetuned-fim-model-integration

Conversation

@sagtanih

@sagtanih sagtanih commented May 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Context

  1. Adds support for finetuned FIM models hosted on Fireworks. This PR adds the 4 variants and the traffic is routed to the appropriate model.
  2. Client side pull request: https://github.com/sourcegraph/cody/pull/4245

Test plan

curl -vS -X POST http://localhost:9992/v1/completions/fireworks -H 'Authorization: bearer <SGD_TOKEN>' -d '{"stream":false,"max_tokens":50, "model": "fim-fine-tuned-model-variant-1", "stop_sequences": ["\n\n"], "prompt": "const value = ", "stream":false}' -H 'X-sourcegraph-feature: code_completions'

curl -vS -X POST http://localhost:9992/v1/completions/fireworks -H 'Authorization: bearer <SGD_TOKEN>' -d '{"stream":false,"max_tokens":50, "model": "fim-fine-tuned-model-variant-2", "stop_sequences": ["\n\n"], "prompt": "const value = ", "stream":false}' -H 'X-sourcegraph-feature: code_completions'

curl -vS -X POST http://localhost:9992/v1/completions/fireworks -H 'Authorization: bearer <SGD_TOKEN>' -d '{"stream":false,"max_tokens":50, "model": "fim-fine-tuned-model-variant-3", "stop_sequences": ["\n\n"], "prompt": "const value = ", "stream":false}' -H 'X-sourcegraph-feature: code_completions'

curl -vS -X POST http://localhost:9992/v1/completions/fireworks -H 'Authorization: bearer <SGD_TOKEN>' -d '{"stream":false,"max_tokens":50, "model": "fim-fine-tuned-model-variant-4", "stop_sequences": ["\n\n"], "prompt": "const value = ", "stream":false}' -H 'X-sourcegraph-feature: code_completions'

@cla-bot cla-bot Bot added the cla-signed label May 22, 2024
@sagtanih sagtanih requested a review from rafax May 22, 2024 06:05
sagtanih
sagtanih commented May 22, 2024
View reviewed changes
Comment thread internal/completions/client/fireworks/fireworks.go Outdated

payload := fireworksRequest{
Model: requestParams.Model,
Model: c.updateModelStringIfFinetunedModelId(requestParams.Model),

@sagtanih sagtanih May 22, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @rafax
My local cody gateway setup is not working, can you please verify this re-routing logic for the fine tuning model string.

@rafax

rafax commented May 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

curl -vS -X POST http://localhost:9992/v1/completions/fireworks -H 'Authorization: bearer <SGD_TOKEN>' -d '{"stream":false,"max_tokens":50, "model": "cody-autocomplete-fim-finetuned-model-variant1", "stop_sequences": ["\n\n"], "prompt": "const value = ", "stream":false}' -H 'X-sourcegraph-feature: code_completions'

You want fim-fine-tuned-model-variant-1 as model name (no cody-autocomplete, with a - in fine-tuned, with a - before 1).

@sagtanih

sagtanih commented May 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

curl -vS -X POST http://localhost:9992/v1/completions/fireworks -H 'Authorization: bearer <SGD_TOKEN>' -d '{"stream":false,"max_tokens":50, "model": "cody-autocomplete-fim-finetuned-model-variant1", "stop_sequences": ["\n\n"], "prompt": "const value = ", "stream":false}' -H 'X-sourcegraph-feature: code_completions'

You want fim-fine-tuned-model-variant-1 as model name (no cody-autocomplete, with a - in fine-tuned, with a - before 1).

Ah yes, sorry I did not run it since my local gateway is not working, I updated the commands in the description

sagtanih
sagtanih commented May 22, 2024
View reviewed changes
Comment thread internal/completions/types/types.go Outdated
Model string `json:"model,omitempty"`
Stream *bool `json:"stream,omitempty"`
Logprobs *uint8 `json:"logprobs"`
LanguageId string `json:"languageId,omitempty"`

@sagtanih sagtanih May 22, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @rafax
I added this new parameter for language level routing to fireworks models. Although it should empty by default, could you confirm this shouldn't break any existing functionality ?

@sagtanih sagtanih May 22, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rafax do we still need changes here after shifting them to fireworks ?

@rafax rafax May 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removed

rafax
rafax approved these changes May 22, 2024
View reviewed changes
Comment thread internal/completions/client/fireworks/fireworks.go Outdated
return dec.Err()
}

func (c *fireworksClient) updateModelStringIfFinetunedModelId(languageId string, model string) string {

@rafax rafax May 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that this code is in Sourcegraph backend (no cody-gateway in the path), so it will not be executed for PLG requests (which bypass sourcegraph.com). If you want this to apply to PLG, you need to modify cmd/cody-gateway/internal/httpapi/completions/fireworks.go (ex. pickStarCoderModel function)

@sagtanih sagtanih May 22, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @rafax do we need this from here ?

@rafax rafax May 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removed

rafax
rafax suggested changes May 22, 2024
View reviewed changes

@rafax rafax left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move code to Cody Gateway

@rafax rafax mentioned this pull request May 22, 2024
Merged
sagtanih
sagtanih commented May 22, 2024
View reviewed changes
Comment thread cmd/cody-gateway/internal/httpapi/completions/fireworks.go Outdated
}

body.Model = pickStarCoderModel(body.Model, f.config)
body.Model = pickFineTunedModel(body.Model, body.LanguageID)

@sagtanih sagtanih May 22, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @rafax
did not notice this before merging the PR, but doing this body.LanguageID = "" in line 142 for the condition body.LanguageID will always make languageId empty. I tested it locally and it was always returning the all language completions. Adding a minor fix for this, which I tested locally.

@rafax rafax May 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right, I updated the E2E tests to catch similar issues (verifying we resolve the right model)

@sagtanih sagtanih merged commit f12393d into main May 22, 2024
@sagtanih sagtanih deleted the hitesh/finetuned-fim-model-integration branch May 22, 2024 19:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@janhartman janhartman Awaiting requested review from janhartman

@rishabhmehrotra rishabhmehrotra Awaiting requested review from rishabhmehrotra

1 more reviewer

@rafax rafax rafax approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sagtanih @rafax