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 self-hosted actions to periodic tests #2929

Merged

Conversation

BradyPlanden
Copy link
Member

@BradyPlanden BradyPlanden commented May 5, 2023

Description

This updates the run_periodic_tests.yml workflow to include the self-hosted m-series runner.

Fixes #2884

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist:

  • No style issues: $ pre-commit run (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
  • All tests pass: $ python run-tests.py --all
  • The documentation builds: $ python run-tests.py --doctest

You can run unit and doctests together at once, using $ python run-tests.py --quick.

Further checks:

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (5e28bb9) 99.71% compared to head (1544a7e) 99.71%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2929   +/-   ##
========================================
  Coverage    99.71%   99.71%           
========================================
  Files          248      248           
  Lines        18749    18749           
========================================
  Hits         18695    18695           
  Misses          54       54           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@BradyPlanden BradyPlanden changed the title Add self-hosted action to test_on_push.yml Add self-hosted actions to periodic tests Jun 22, 2023
@BradyPlanden
Copy link
Member Author

BradyPlanden commented Jun 23, 2023

This is ready to merge now @martinjrobins. Here's an overview:

  • The runner creates and destroys the python environment on completion, giving us similar functionality to the GitHub hosted runners.
  • The runner utilised pyenv + virtualenvs to create and destroy virtual environments on each deployment.
  • Pyenv will pull the most recent python release for each 3.X specified in python-version
  • At the moment, the python versions run sequentially. If our organisation has access to enterprise features, we can change this to parallel instances by grouping multiple runner instances on the mini, which would greatly speed up the completion time (probably doesn't matter for scheduled tests, but might for others if we continue with the runner longer-term)
  • The integration tests currently fail on m-series hardware (as per [1])

We will be relocating the runner onto a separate network in the very near future, but it would be good to validate this workflow ahead of that, so it might just go offline for a day or two during that migration.

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

Merging the nox PR generated conflicts with a lot of open PRs 😬

.github/workflows/run_periodic_tests.yml Outdated Show resolved Hide resolved
.github/workflows/run_periodic_tests.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

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

looks great, thanks @BradyPlanden, couple of suggestions below, and add @Saransh-cpp's changes as well

.github/workflows/run_periodic_tests.yml Outdated Show resolved Hide resolved
.github/workflows/run_periodic_tests.yml Outdated Show resolved Hide resolved
@BradyPlanden BradyPlanden force-pushed the apple-runner-workflow branch 2 times, most recently from 90e5bc6 to 5e28bb9 Compare July 3, 2023 12:20
@BradyPlanden
Copy link
Member Author

Thanks @martinjrobins and @Saransh-cpp, this is updated now and should be good for another review 👍

Copy link
Contributor

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

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

Thanks @BradyPlanden , happy to merge this now :)

@BradyPlanden BradyPlanden merged commit 1a012e7 into pybamm-team:develop Jul 10, 2023
@BradyPlanden BradyPlanden deleted the apple-runner-workflow branch July 10, 2023 15:34
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.

Apple M-Series Self-Hosted Runner
3 participants