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

General PR slow CI #30540

Merged
merged 2 commits into from
Apr 30, 2024
Merged

General PR slow CI #30540

merged 2 commits into from
Apr 30, 2024

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Apr 29, 2024

What does this PR do?

Extend #30341 to handle general PR slow CI. Now, it could handle

  • new model pull request
  • or a pull request with the (latest) commit have the form [run-slow]bert,gpt2
    • the prefix could be [run-slow], [run_slow] or [run slow]
    • the remaining part should be comma-separated like bert,gpt2 or bert, gpt2 etc.
      • there is no syntax check or error handling of the wrong format (we can add them later if we find necessary)

The script and workflow file names (and their methods or job/step names too) to reflect they handle more general cases instead of just new model cases.

An example run:

  • (I deliberate make a failing tests)
  • it's trigger with the commit message [run-slow]bert,gpt2

IMPORTANT: the label to add in order to trigger this CI is run-slow.

single-model-run-slow is no longer used.

@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.

@ydshieh ydshieh force-pushed the run_pr_slow_ci branch 2 times, most recently from 27c66e0 to 28e438c Compare April 29, 2024 13:05
@ydshieh ydshieh marked this pull request as ready for review April 30, 2024 14:12
@@ -4,6 +4,11 @@ on:
pull_request:
paths:
- "src/transformers/models/*/modeling_*.py"
- "tests/models/*/test_*.py"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If nothing changed in those paths, the CI won't be triggered at all
(even if the commit message specifies some models)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep for now - I'm not sure whether we want this. It's good for the auto model selection, but I can see as being annoying for the run-slow case: I might update e.g. modeling_attn_mask_utils.py and want to be able to easily run the slow tests on a subset of important models

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we will see 🚀 !


concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To cancel the CI runs trigger by previous commits of the same PR

python -m pip install GitPython
echo "new_model=$(python utils/check_if_new_model_added.py | tail -n 1)" >> $GITHUB_OUTPUT
python utils/pr_slow_ci_models.py --commit_message "${{ env.commit_message }}" | tee output.txt
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now that (renamed) script will give the models to run (either new model or from the commit message)

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.

Amazing ❤️ Thanks for adding this so quickly - looking forward to trying it out!

utils/pr_slow_ci_models.py Outdated Show resolved Hide resolved
@@ -4,6 +4,11 @@ on:
pull_request:
paths:
- "src/transformers/models/*/modeling_*.py"
- "tests/models/*/test_*.py"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep for now - I'm not sure whether we want this. It's good for the auto model selection, but I can see as being annoying for the run-slow case: I might update e.g. modeling_attn_mask_utils.py and want to be able to easily run the slow tests on a subset of important models

@ydshieh ydshieh merged commit 87927b2 into main Apr 30, 2024
8 checks passed
@ydshieh ydshieh deleted the run_pr_slow_ci branch April 30, 2024 19:05
@ydshieh ydshieh mentioned this pull request May 2, 2024
itazap pushed a commit that referenced this pull request May 14, 2024
* More general PR slow CI

* Update utils/pr_slow_ci_models.py

Co-authored-by: amyeroberts <[email protected]>

---------

Co-authored-by: ydshieh <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants