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

Add workflow to automate durations files updates #6247

Open
wants to merge 47 commits into
base: master
Choose a base branch
from

Conversation

mudit2812
Copy link
Contributor

@mudit2812 mudit2812 commented Sep 10, 2024

  • Added a workflow update-durations.yml that uses stored durations artifacts to open a PR containing the updated durations (example workflow run here, example PR here)
    • The workflow will run on a weekly schedule
  • Moved durations files to .github/durations folder.
  • Change step in tests workflow that uploads stable dependencies to only run if the workflow was triggered by schedule. This change is made to avoid opening a PR for stable dependency updates every time there is a push to master.
  • Update tests workflow to use GitHub CLI instead of the repo-sync/pull-request action, which is archived.

Comments below with implementation details.

[sc-72992]

Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@@ -11,7 +11,7 @@ on:
required: true
type: string
pipeline_mode:
description: The pipeline mode can be unit-tests, benchmarks, or reference-benchmarks
description: The pipeline mode can be unit-tests, update-durations, benchmarks, or reference-benchmarks
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This new pipeline mode is used to selectively run jobs that are needed for updating the durations files.

Comment on lines -45 to -49
pytest_store_durations:
description: Whether to store artifacts for test durations
required: false
type: boolean
default: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this in favour of the new update-durations pipeline mode.

@@ -212,7 +207,7 @@ jobs:
|| fromJSON(needs.setup-ci-load.outputs.matrix-max-parallel).default
}}
matrix:
group: [1, 2, 3]
group: ${{ inputs.pipeline_mode == 'update-durations' && fromJSON('[1]') || fromJSON('[1, 2, 3]') }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't split the jobs for tensorflow, jax, and core tests when updating durations. This way, rather than generating multiple artifacts for each job, we generate a single unified one that can be used as the updated durations file.

@@ -129,7 +124,7 @@ jobs:
repository: PennyLaneAI/pennylane

- name: Determine benchmark name
if: ${{ inputs.pipeline_mode != 'unit-tests' }}
if: ${{ !contains(fromJSON('["unit-tests", "update-durations"]'), inputs.pipeline_mode) }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skip benchmarks pipeline related steps when not needed.

Comment on lines 179 to 181
PYTEST_STORE_ARGS: ${{ inputs.pytest_store_durations == true && '--store-durations --clean-durations' || '' }}
PYTEST_STORE_ARGS: ${{ inputs.pipeline_mode == 'update-durations' && '--store-durations --clean-durations' || '' }}
run: |
if [[ "$PIPELINE_MODE" =~ .*"benchmarks".* ]]; then
echo "args=$PYTEST_BENCHMARKS_ARGS $PYTEST_ADDITIONAL_ARGS" >> $GITHUB_OUTPUT
elif [[ "$PIPELINE_MODE" == "update-durations" ]]; then
echo "args=$PYTEST_PARALLELISE_ARGS $PYTEST_DURATIONS_ARGS $PYTEST_STORE_ARGS" >> $GITHUB_OUTPUT
else
echo "args=$PYTEST_COVERAGE_ARGS $PYTEST_PARALLELISE_ARGS $PYTEST_ADDITIONAL_ARGS $PYTEST_DURATIONS_ARGS $PYTEST_STORE_ARGS" >> $GITHUB_OUTPUT
echo "args=$PYTEST_COVERAGE_ARGS $PYTEST_PARALLELISE_ARGS $PYTEST_ADDITIONAL_ARGS $PYTEST_DURATIONS_ARGS" >> $GITHUB_OUTPUT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If using the update-durations pipeline, don't include codecov arguments or benchmarks arguments.

Comment on lines 195 to 201
- name: Upload Durations file as artifact
if: ${{ inputs.pytest_store_durations == true }}
if: ${{ inputs.pipeline_mode == 'update-durations' }}
uses: actions/upload-artifact@v4
with:
name: ${{ inputs.job_name }}-durations.json
name: durations-${{ inputs.job_name }}
path: ${{ inputs.pytest_durations_file_path }}
include-hidden-files: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only upload durations as artifacts if using the update-durations pipeline mode. Also changed the artifact name to start with durations- to be easier to search for when downloading the artifacts

@@ -0,0 +1,102 @@
name: Update Durations Files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This workflow will use the update-durations pipeline mode for running the tests. Core, tf, and jax tests will be run (since those are the only ones with durations files associated with them), the durations files will be stored as artifacts, and then the workflow will open a PR with the updated durations files.

The workflow will run on a weekly schedule, and will also be available using a manual trigger.

cancel-in-progress: true

env:
DURATIONS_BRANCH: bot/durations-update
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Branch from which durations file update PR will be opened.

Comment on lines +43 to +45
pipeline_mode: 'update-durations'
run_lightened_ci: true
skip_ci_test_jobs: 'torch-tests,autograd-tests,all-interfaces-tests,external-libraries-tests,qcut-tests,qchem-tests,gradients-tests,data-tests,device-tests'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We run lightened CI so that we only run the tests with python 3.10, and the listed tests are skipped (everything except for tf, jax, and core).

Comment on lines 48 to 102
upload-durations-files:
needs: update-durations
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
with:
fetch-depth: 0
ref: master
# ref: ${{ github.ref }}

- name: Prepare local repo
run: |
git config user.name "GitHub Actions Bot"
git config user.email "<>"
if git ls-remote --exit-code origin "refs/heads/${{ env.DURATIONS_BRANCH }}"; then
git checkout "${{ env.DURATIONS_BRANCH }}"
else
git checkout -b "${{ env.DURATIONS_BRANCH }}"
fi

- name: Download artifacts
uses: actions/download-artifact@v4
with:
pattern: durations-*
path: .github/durations/
merge-multiple: true

- name: Stage changes
run: |
git add .github/durations/
git commit -m "Update durations files"
git push -f --set-upstream origin "${{ env.DURATIONS_BRANCH }}"

# Create PR to master
- name: Create pull request
uses: repo-sync/pull-request@v2
with:
source_branch: "${{ env.DURATIONS_BRANCH }}"
destination_branch: "master"
github_token: "${{ secrets.GITHUB_TOKEN }}"
pr_title: "Update test duration files"
# if updating the pr_body, please also update the same text in the docs.yml file
pr_body: |
Automatic update of test duration files.

Because bots are not able to trigger CI on their own, please do so by pushing an empty commit to this branch using the following command:

```
git commit --allow-empty -m 'trigger ci'
```

Alternatively, wait for this branch to be out-of-date with master, then just use the "Update branch" button!
pr_allow_empty: false
pr_draft: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Checkout out pennylane
  • If DURATIONS_BRANCH exists, checkout to it, otherwise create DURATIONS_BRANCH and checkout to it.
  • Download durations files to the .github/durations directory
  • Commit changes and publish the branch.
  • Open a PR

Comment on lines 69 to 86
- name: Create pull request
if: steps.changed.outputs.has_changes != '0'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_TITLE: 'Update test duration files'
PR_BODY: |
Automatic update of stable requirement files to snapshot valid python environments.

Because bots are not able to trigger CI on their own, please do so by pushing an empty commit to this branch using the following command:

```
git commit --allow-empty -m 'trigger ci'
```

Alternatively, wait for this branch to be out-of-date with master, then just use the 'Update branch' button.
run: |
gh_pr_up() { gh pr create "$@" || gh pr edit "$@" }
gh_pr_up --title "$PR_TITLE" --body "$PR_BODY"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This step assumes that any older PRs for updating durations have either been merged or are still open. If the PRs are closed without the branch having been deleted, this step will fail. In that case, the artifacts generated by the action can be used to manually update the durations files.

Originally I was using the repo-sync/pull-request action as used in other pennylane workflows. However, I noticed that the action is archived, so I'm using Github CLI instead as suggested by the author of the original action. I tried to add the core team as a reviewer, but it kept failing, and seems to be related to this issue, which seems to stem from not having the needed permissions to do so with the github token.

For now, we could probably get away with just assigning people rather than teams.

# Scheduled trigger every Saturday at 2:47am UTC
schedule:
- cron: '47 2 * * 6'
workflow_dispatch:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a workflow_dispatch trigger so that we can manually trigger the workflow if any PR massively imbalances the load.

@mudit2812
Copy link
Contributor Author

Most recent PR with updated durations here.

@mudit2812 mudit2812 marked this pull request as ready for review September 13, 2024 20:58
Copy link

codecov bot commented Sep 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.58%. Comparing base (5f4169a) to head (f1d55fa).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6247      +/-   ##
==========================================
- Coverage   99.63%   99.58%   -0.06%     
==========================================
  Files         445      443       -2     
  Lines       42433    42230     -203     
==========================================
- Hits        42278    42053     -225     
- Misses        155      177      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

1 participant