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

Split the github workflow in CI and CD #1063

Merged
merged 34 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
033b40d
First version
famura Mar 20, 2024
f89a91a
Deleted the old CI
famura Mar 20, 2024
aaad036
Do not run on draft PRs
famura Mar 20, 2024
fe86809
Deleted comment
famura Mar 20, 2024
14cb7d4
Added --exitfirst
famura Mar 20, 2024
4e9be3c
Commented out if for testing the workflow
famura Mar 20, 2024
3b35f9c
Moved the pull_request tag from CI to CD
famura Mar 20, 2024
3916b90
Changing on push and pull_request for CI and CD
famura Mar 20, 2024
0a37af8
More specific concurrency
famura Mar 20, 2024
55587fb
Debugging why CD is not triggered
famura Mar 20, 2024
a999891
Moved if clause
famura Mar 20, 2024
1e0e73a
Bug fix
famura Mar 20, 2024
f08e7b5
Changed flags for code cov
famura Mar 20, 2024
5f8266d
Added -n auto from recent PR
famura Mar 21, 2024
b4bee53
Installing pytest cov with the dev option
famura Mar 21, 2024
4c6987b
Updated testing and cov options
famura Mar 21, 2024
c3b3b79
lfs is not used
famura Mar 21, 2024
499410b
Revert to pre-commit hooks as a separate job
famura Mar 21, 2024
83a276f
First try of the codecov.yml
famura Mar 21, 2024
79d3243
Format fix
famura Mar 21, 2024
ca0b856
No tokes was found, is this due to the codecov.yml?
famura Mar 21, 2024
267e90c
The token was still not found
famura Mar 21, 2024
659d42a
Before this commit a token was found
famura Mar 21, 2024
6f9c98a
Test doc bulding
famura Mar 22, 2024
e05d7df
Docs workflow worked, moved it to CD
famura Mar 22, 2024
f3a15bf
More detailed caching key
famura Mar 25, 2024
67bfef5
Run fast tests for pushes and fast+slow for PRs
famura Mar 25, 2024
283fd87
Testing double exec
famura Mar 25, 2024
6036a3b
Revert since the last commit is only executed when the PR is merged
famura Mar 25, 2024
aaec3f7
Add fallback keys if full key is not available
Baschdl Mar 25, 2024
6958f94
Do not run the slow tests in CI
famura Mar 25, 2024
8be7169
Fixed typo
famura Mar 25, 2024
b472ab0
Add a workflow for manual selection of markers
famura Mar 25, 2024
fc53abb
Added coverage report for fast CI back in
famura Mar 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/codecov.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# The official docs
# https://docs.codecov.io/docs/codecov-yaml
# https://docs.codecov.com/docs/common-recipe-list

# Check if this file is valid
# cd PATH_TO/sbi/.github
# curl -X POST --data-binary @codecov.yml https://codecov.io/validate

ignore:
- "sbi/examples"

coverage:
status:
project:
default:
target: 70% # the required coverage value
threshold: 2% # allow the coverage to drop by X%, and posting a success status
if_ci_failed: error # will set the status to success only if the CI is successful, alternative: success
patch: # about the individual commit
default:
target: 50% # minimum coverage ratio that the commit must meet to be considered a success
threshold: 2% # allow the coverage to drop by X%, and posting a success status
if_ci_failed: error # will set the status to success only if the CI is successful, alternative: success

comment:
layout: "diff, flags, files"
behavior: default # update if exists, otherwise post new
require_changes: false # if true, only post the comment if coverage changes
78 changes: 78 additions & 0 deletions .github/workflows/cd.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
name: Continuous Deployment

on:
push:
branches: [main]
workflow_dispatch:

defaults:
run:
shell: bash
famura marked this conversation as resolved.
Show resolved Hide resolved

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
famura marked this conversation as resolved.
Show resolved Hide resolved
cancel-in-progress: true

jobs:
pre-commit:
name: ruff and hooks.
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.8'
- uses: pre-commit/action@v3.0.1
with:
extra_args: --all-files --show-diff-on-failure

cd:
name: CD
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
lfs: false

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.8'
Copy link
Contributor

Choose a reason for hiding this comment

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

How about testing in CD for more than only the oldest Python version we currently support? Ideally all but we could also limit it.

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 would argue against this since the purpose of CD is not a (max. board) compatibility test. One could do this in the CI workflow as it is preferable to let these things fail fast there. Doing it at both places just doubles the effort without a real gain. It is highly likely that if sth is failing due to the wrong Python version, it is affecting the "not slow" 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.

I don't think we should do that in the CD workflow as its purpose is a bit of a different one (computing max coverage, doc, etc.). Moreover, I think that it is very unlikely that there is a failure introduced due to Python version incompatibility which only occurs in the slow tests and not the others.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @michaeldeistler had a strong opinion against increasing the runtime of our CI. In theory (with unlimited runners) adding more Python versions to CI will not increase the runtime as they're running in parallel but in cases like the hackathon this can make a difference. I don't have a strong opinion where to do it and on how many versions but I would be in favor of testing more than just one version somewhere.

Github Actions also has the option to schedule Actions like cron jobs, e.g. to run on multiple versions at least once per day.

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've added a workflow that can be run manually to check all tests (or another marker) if desired


- name: Cache dependency
id: cache-dependencies
uses: actions/cache@v4
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -e .[dev]

- name: Check types with pyright
run: |
pyright sbi
famura marked this conversation as resolved.
Show resolved Hide resolved

- name: Run the fast and the slow CPU tests with coverage
run: |
pytest -v -x -n auto -m "not gpu" --cov=sbi --cov-report=xml tests/

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v4-beta
with:
env_vars: OS,PYTHON
file: ./coverage.xml
flags: unittests
name: codecov-sbi-all-cpu
token: ${{ secrets.CODECOV_TOKEN }}

- name: Check doc building
run: |
jupyter nbconvert --to markdown tutorials/*.ipynb --output-dir docs/tutorial/
jupyter nbconvert --to markdown examples/*.ipynb --output-dir docs/examples/
mkdocs build -f docs/mkdocs.yml --site-dir site
81 changes: 81 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
name: Continuous Integration

on: [pull_request, workflow_dispatch]

defaults:
run:
shell: bash

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
cancel-in-progress: true

jobs:
pre-commit:
name: ruff and hooks.
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.8'
- uses: pre-commit/action@v3.0.1
with:
extra_args: --all-files --show-diff-on-failure

ci:
name: CI
runs-on: ubuntu-latest
if: |
github.event_name == 'push' ||
(github.event_name == 'pull_request' && github.event.pull_request.draft == false)
famura marked this conversation as resolved.
Show resolved Hide resolved
strategy:
fail-fast: false
matrix:
python-version: ['3.8']
torch-version: ['1.11', '2.2']

steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
lfs: false

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

- name: Cache dependency
id: cache-dependencies
uses: actions/cache@v4
with:
path: ~/.cache/pip
famura marked this conversation as resolved.
Show resolved Hide resolved
key: ${{ runner.os }}-pip-${{ matrix.python-version }}-${{ matrix.torch-version }}$
restore-keys: |
${{ runner.os }}-pip-${{ matrix.python-version }}-
${{ runner.os }}-pip-

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install torch==${{ matrix.torch-version }} --extra-index-url https://download.pytorch.org/whl/cpu
pip install -e .[dev]

- name: Check types with pyright
run: |
pyright sbi

- name: Run the fast CPU tests with coverage
run: |
pytest -v -x -n auto -m "not slow and not gpu" --cov=sbi --cov-report=xml tests/

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v4
with:
env_vars: OS,PYTHON
file: ./coverage.xml
flags: unittests
name: codecov-sbi-fast-cpu
token: ${{ secrets.CODECOV_TOKEN }}
65 changes: 65 additions & 0 deletions .github/workflows/manual_test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
name: Manual-Test

on:
workflow_dispatch:
inputs:
pytest-marker:
description: "Combination of markers to restrict the tests, use '' to run all tests."
type: choice
options:
- 'not slow and not gpu'
- 'not gpu'
- 'not slow'
- ''
default: ''
required: true

defaults:
run:
shell: bash

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
cancel-in-progress: true

jobs:
test:
name: manual-test
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ['3.8']
torch-version: ['1.11', '2.2']

steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
lfs: false

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

- name: Cache dependency
id: cache-dependencies
uses: actions/cache@v4
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ matrix.python-version }}-${{ matrix.torch-version }}$
restore-keys: |
${{ runner.os }}-pip-${{ matrix.python-version }}-
${{ runner.os }}-pip-

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install torch==${{ matrix.torch-version }} --extra-index-url https://download.pytorch.org/whl/cpu
pip install -e .[dev]

- name: Run the selected tests without coverage
run: |
pytest -v -x -m ${{ inputs.pytest-marker }} tests/
71 changes: 0 additions & 71 deletions .github/workflows/tests.yml

This file was deleted.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ dev = [
"ruff>=0.3.3",
# Test
"pytest",
"pytest-cov",
famura marked this conversation as resolved.
Show resolved Hide resolved
"pytest-xdist",
"torchtestcase",
]
Expand Down