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.

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

Fix issue when generating distros #755

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

terrytangyuan
Copy link
Collaborator

Addressed comment #723 (comment).

cc @yanxi0830

I am not 100% sure if the diff is correct though but this is the result of running python llama_stack/scripts/distro_codegen.py.

@ashwinb
Copy link
Contributor

ashwinb commented Jan 14, 2025

@dineshyv could you approve this PR? I am not sure if the changes to the run-with-safety.yaml files are good or not

@@ -95,62 +95,20 @@ metadata_store:
db_path: ${env.SQLITE_STORE_DIR:~/.llama/distributions/fireworks}/registry.db
models:
- metadata: {}
model_id: meta-llama/Llama-3.1-8B-Instruct
model_id: ${env.INFERENCE_MODEL}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this run-with-safety template shouldn't be changed.

cc @vladimirivic Could you help take a look? I think this is introduced incorrectly in

"run-with-safety.yaml": RunConfigSettings(
provider_overrides={
"inference": [
inference_provider,
embedding_provider,
],
"memory": [memory_provider],
"safety": [
Provider(
provider_id="llama-guard",
provider_type="inline::llama-guard",
config={},
),
Provider(
provider_id="code-scanner",
provider_type="inline::code-scanner",
config={},
),
.

It should follow smt like together's run-with-safety template: https://github.com/meta-llama/llama-stack/blob/91907b714e825a1bfbca5271e0f403aab5f10752/llama_stack/templates/together/together.py#L120C32-L141

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I rebased and re-generated the files again. Not sure if it incorported the recent fixes correctly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it pulled changes in #766.

Copy link
Contributor

Choose a reason for hiding this comment

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

The fireworks templates have been fixed in #766 . But I think the vllm run.yaml without tool_runtime still suggest you might have an older version? Could you help double check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like there are indeed some issues with my local python env. I think I fixed them now. Just pushed an update

Signed-off-by: Yuan Tang <[email protected]>
@terrytangyuan terrytangyuan changed the title Fix issue when generating vLLM distros Fix issue when generating distros Jan 15, 2025
@yanxi0830
Copy link
Contributor

LG!

@ashwinb ashwinb merged commit 300e6e2 into meta-llama:main Jan 15, 2025
2 checks passed
@terrytangyuan terrytangyuan deleted the fix-typo-dis branch January 15, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants