FIX: Fix corner-case issue with the important models workflow #30212
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| runs-on: [single-gpu, nvidia-gpu, a10, ci] | ||
| container: | ||
| image: huggingface/transformers-all-latest-gpu | ||
| options: --gpus all --privileged --ipc host -v /mnt/cache/.cache/huggingface:/mnt/cache/ | ||
| if: ${{ needs.get_modified_models.outputs.matrix != '[]' && needs.get_modified_models.outputs.matrix != '' }} | ||
| if: ${{ needs.get_modified_models.outputs.matrix != '[]' && needs.get_modified_models.outputs.matrix != '' && fromJson(needs.get_modified_models.outputs.matrix)[0] != null }} |
There was a problem hiding this comment.
That's the most efficient condition i could use, simply converting to an array and trying to access to the first element seemed to work
There was a problem hiding this comment.
Are we always going to be able to safely index on 0 here?
There was a problem hiding this comment.
yes i think so , per my understanding if the array is empty fromJson(needs.get_modified_models.outputs.matrix)[0] will give null , so the check fromJson(needs.get_modified_models.outputs.matrix)[0] != null circumvents the case where the array is empty
There was a problem hiding this comment.
Great - thanks for confirming!
younesbelkada
changed the title
|
@younesbelkada Could you include in the PR description what the corner case is? I can see this is a fix but it's not clear what 's being addressed |
|
Thanks @amyeroberts ! Sorry, yes - I just added some description, let me know what do you think - here is also an example of that corner case: https://huggingface.slack.com/archives/C06L2SGMEEA/p1713186513863159 |
| runs-on: [single-gpu, nvidia-gpu, a10, ci] | ||
| container: | ||
| image: huggingface/transformers-all-latest-gpu | ||
| options: --gpus all --privileged --ipc host -v /mnt/cache/.cache/huggingface:/mnt/cache/ | ||
| if: ${{ needs.get_modified_models.outputs.matrix != '[]' && needs.get_modified_models.outputs.matrix != '' }} | ||
| if: ${{ needs.get_modified_models.outputs.matrix != '[]' && needs.get_modified_models.outputs.matrix != '' && fromJson(needs.get_modified_models.outputs.matrix)[0] != null }} |
There was a problem hiding this comment.
Are we always going to be able to safely index on 0 here?
|
Thanks for the review ! |
…gface#30212) * Update push-important-models.yml * dummy commit * Update modeling_bark.py * test * test * test * another test * another test * test * final test * final test * test * another test * test * test * another test * test llama * revert everything * remove echo
* Update push-important-models.yml * dummy commit * Update modeling_bark.py * test * test * test * another test * another test * test * final test * final test * test * another test * test * test * another test * test llama * revert everything * remove echo
* Update push-important-models.yml * dummy commit * Update modeling_bark.py * test * test * test * another test * another test * test * final test * final test * test * another test * test * test * another test * test llama * revert everything * remove echo
As per title
Before this PR, there was a bug when `models/xxx files where modified because a check for empty arrays on the GitHub action was being bypassed leading to the action being run unexpectedly such as: https://github.com/huggingface/transformers/actions/runs/8688185234 - this PR fixes it