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

Update daily ci to use new cluster #33627

Merged
merged 2 commits into from
Sep 20, 2024
Merged

Update daily ci to use new cluster #33627

merged 2 commits into from
Sep 20, 2024

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Sep 20, 2024

What does this PR do?

And re-enable daily (which was disabled in #33136)

I haven't had a final confirmation from @glegendre01 , but let's take a chance to run it during this weekend.

Comment on lines +101 to +116
- name: Set `machine_type` for report and artifact names
working-directory: /transformers
shell: bash
run: |
echo "${{ inputs.machine_type }}"

if [ "${{ inputs.machine_type }}" = "aws-g4dn-2xlarge-cache" ]; then
machine_type=single-gpu
elif [ "${{ inputs.machine_type }}" = "aws-g4dn-12xlarge-cache" ]; then
machine_type=multi-gpu
else
machine_type=${{ inputs.machine_type }}
fi

echo "$machine_type"
echo "machine_type=$machine_type" >> $GITHUB_ENV
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The new cluster is named aws-g4dn-2xlarge-cache or aws-g4dn-12xlarge-cache, but we need single-gpu and/or multi-gpu for the report file name or artifact name.

Comment on lines +5 to +7
repository_dispatch:
schedule:
- cron: "17 2 * * *"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

re-enable

@@ -102,7 +103,7 @@ jobs:
strategy:
fail-fast: false
matrix:
machine_type: [single-gpu, multi-gpu]
machine_type: [aws-g4dn-2xlarge-cache, aws-g4dn-12xlarge-cache]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

new cluster

@HuggingFaceDocBuilderDev

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.

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for updating!

Only comment is that the mapping logic from the aws cluster name to "x-gpu" is very repetitive. I don't know of an obvious way to abstract this out in the yaml, but it would be cool if we could :)

@ydshieh
Copy link
Collaborator Author

ydshieh commented Sep 20, 2024

Thanks for updating!

Only comment is that the mapping logic from the aws cluster name to "x-gpu" is very repetitive. I don't know of an obvious way to abstract this out in the yaml, but it would be cool if we could :)

Yes I agree. I will think of it but let's try to run the CI during this weekend and potentially give feedbacks to the infra team on Monday 🙏 Thank you for the review.

@ydshieh ydshieh merged commit 75c878d into main Sep 20, 2024
9 checks passed
@ydshieh ydshieh deleted the update_daily_ci branch September 20, 2024 19:05
BernardZach pushed a commit to BernardZach/transformers that referenced this pull request Dec 5, 2024
* update

* re-enable daily CI

---------

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants