-
Notifications
You must be signed in to change notification settings - Fork 20
Added support for Whisper external model download (#544) #546
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
Conversation
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
Hi @kamieyy, Thanks for the effort in raising this PR. To align with the structure and practices we follow in this repo, a few changes would be needed before we can move forward. Specifically, for model and dataset downloads, we typically expose them through the meta configuration, which allows for more consistent automation. You can refer to how this was handled for Llama2 here for reference. Since you're new to the repo, I’d highly recommend going through the MLCFlow documentation to get a clearer picture of how the execution flow is structured. Feel free to revise the PR based on this. Let me know if you need help, I'll be happy to guide you through it! |
Hi @anandhu-eng, thanks for the review! I've made changes to the meta.yaml to
Please let me know if anything needs further adjustment or cleanup. And, thanks again for pointing me in the right direction. |
7d122a5
to
4934283
Compare
The automation framework automation/script/module.py dep_tags_list = dep.get('tags').split(",") expects every dependency to have a tags string, otherwise it throws an error.
recheck |
- MLC_OUTDIRNAME | ||
names: | ||
- whisper-outdir-setup | ||
tags: setup,ml-model |
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.
Do we have this mlc script?
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.
@arjunsuresh Thanks for pointing it out. I've changed it to 'dae' like others.
removed - MLC_OUTDIRNAME names: - whisper-outdir-setup tags: setup,ml-model
There are currently two CI errors: one related to insufficient disk space on the CI runner (OSError: [Errno 28] No space left on device), and another due to missing 'offline' scenario results for the PointPainting model (SubmissionCheckerError: Offline scenario results not found). Based on my review, I'm not sure if these issues are caused by the changes introduced in this PR. Is there anything I'm missing? |
You can ignore the test failures. 2 of those are due to a pending PR in the inference repository. |
Thanks again @kamieyy ! |
Description
adds support for downloading the OpenAI Whisper large-v3 model from Hugging Face using MLCFlow.
Related Issue
Fixes #544
🧾 PR Checklist
dev
📌 Note: PRs must be raised against
dev
. Do not commit directly tomain
.