-
Notifications
You must be signed in to change notification settings - Fork 26
⏪ revert back to 3-layer micro model #497
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
Changes from all commits
87b339d
7174818
5aeeb88
38acec8
49c5077
e90387e
b194d0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,15 +174,15 @@ jobs: | |
| key: ${{ runner.os }}-hf-model-${{ env.model_key }} | ||
|
|
||
| - name: "Download HF models" | ||
| if: ( steps.changed-src-files.outputs.any_changed == 'true' && steps.cache_restore.outputs.cache-hit != 'true' ) | ||
| if: ( steps.changed-src-files.outputs.any_changed == 'true' && steps.cache_restore.outputs.cache-hit == 'true' ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means you will have both revisions:
Making all the tests work. If you bypass the cache, I reckon some tests will break like in PR #499 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once the 3 layer model is verified, I don't think we need the 4 layer anymore, so we should only be using one revision of the model at all places There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally the fix has to be that we delete the old cache and populate it with the 3 layer revision and merge this PR right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it needs more code/test changes to make sure we don't use the latest/main revision of the tiny granite model anymore/anywhere. Kicking of a test run here that is not also still using the old cached model: #502 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup:
Also, need to not use the same revision key to download the FP8 model 🙄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| run: | | ||
| # We are caching HF models (HF_HUB_CACHE) for reliability rather than speed, since HF downloads are flaky for concurrent jobs. | ||
| # Be careful when adding models to the cache here, as the GHA cache is limited to 10 GB. | ||
| # If a new model is added here, a new hash key is generated. The previous cache blob can then | ||
| # be removed by an admin or can be left to expire after 7 days. | ||
|
|
||
| download_tinygranite() { | ||
| python -c "from transformers import pipeline, AutoTokenizer; pipeline('text-generation', model='$1'); tokenizer=AutoTokenizer.from_pretrained('$1')" | ||
| python -c "from transformers import pipeline; pipeline('text-generation', model='$1', revision='2714578f54cfb744ece40df9326ee0b47e879e03');" | ||
ckadner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| download_roberta_large() { | ||
| python -c "from sentence_transformers import SentenceTransformer; SentenceTransformer('$1')" | ||
|
|
||
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.
temporary change to get GH to download the older revision