Skip to content
Merged
80 changes: 76 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ on:
description: "HubAI API key"
required: true
pull_request:
# `ready_for_review` runs the suites that a draft skips.
types: [opened, synchronize, reopened, ready_for_review]
paths:
- ".python-version"
- "pyproject.toml"
Expand All @@ -74,6 +76,47 @@ permissions:
contents: read

jobs:
# The GPU runner is billed by the minute. This job selects the GPU test
# suites, so that a pull request runs a GPU suite only when the suite
# can find something new:
# - a pull request that changes only the documentation runs no GPU
# suite, see `scripts/only_docstrings_changed.py`;
# - a draft pull request skips the heavy suite.
# A dispatch and a call from another workflow run every suite.
plan:
runs-on: ubuntu-latest
outputs:
gpu_suites: ${{ steps.plan.outputs.gpu_suites }}
code_changed: ${{ steps.plan.outputs.code_changed }}
steps:
- name: Checkout
if: ${{ github.event_name == 'pull_request' && !inputs.train_ref }}
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
with:
ref: ${{ github.head_ref }}
fetch-depth: 0
persist-credentials: false
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

- name: Select the test suites
id: plan
env:
PULL_REQUEST: ${{ github.event_name == 'pull_request' && !inputs.train_ref }}
DRAFT: ${{ github.event.pull_request.draft == true }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
run: |
code_changed=true
gpu_suites='["predefined_light", "predefined_heavy", "combinations", "misc"]'
if [[ "$PULL_REQUEST" == true ]]; then
if python3 scripts/only_docstrings_changed.py "$BASE_SHA"; then
code_changed=false
gpu_suites='[]'
elif [[ "$DRAFT" == true ]]; then
gpu_suites='["predefined_light", "combinations", "misc"]'
fi
fi
echo "code_changed=$code_changed" >> "$GITHUB_OUTPUT"
echo "gpu_suites=$gpu_suites" >> "$GITHUB_OUTPUT"

pre-commit:
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -286,16 +329,21 @@ jobs:

tests:
needs:
- plan
- config-test
- doctests

# The `cpu` suite holds the unit tests and the integration tests
# that do not train, so it runs on a free runner. See
# `pytest_collection_modifyitems` in `tests/conftest.py`.
strategy:
matrix:
suite: [unit, predefined_light, predefined_heavy, combinations, misc]
suite: [cpu]

runs-on: ${{ matrix.suite == 'unit' && 'ubuntu-latest' || 'ubuntu-t4-4core' }}
runs-on: ubuntu-latest

steps:
# `gpu-tests` runs the same steps.
steps: &test-steps
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
with:
Expand Down Expand Up @@ -363,10 +411,34 @@ jobs:
if-no-files-found: ignore
overwrite: true

# The GPU suites start only after the `cpu` suite passes. A failure that
# the `cpu` suite finds then costs no time on the GPU runner. The price
# is a longer run: the `cpu` suite takes about 15 minutes.
gpu-tests:
name: tests (${{ matrix.suite }})
needs:
- plan
- tests
if: ${{ needs.plan.outputs.gpu_suites != '[]' }}

strategy:
matrix:
suite: ${{ fromJSON(needs.plan.outputs.gpu_suites) }}

runs-on: ubuntu-t4-4core

steps: *test-steps

# Coverage is uploaded once, after all the suites have finished. Uploading
# each suite on its own makes Codecov report the partial runs as the total.
# For the same reason, a change of the documentation, which runs the CPU
# suite only, uploads nothing. It cannot change the coverage.
coverage:
needs: tests
needs:
- plan
- tests
- gpu-tests
if: ${{ needs.plan.outputs.code_changed == 'true' }}
runs-on: ubuntu-latest

permissions:
Expand Down
23 changes: 23 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ This command will run all tests and generate HTML coverage report.
> [!TIP]
> You can choose to run only the unit-tests or only the integration tests by adding `-m unit` or `-m integration` to the `pytest` command.

> [!IMPORTANT]
> Mark an integration test with `@pytest.mark.gpu` when it trains, tunes or quantizes a model.
> CI runs these tests on the GPU runner, and all the other tests on a free CPU runner.
> A test without the marker still passes on the CPU, but it no longer covers the GPU code of the training, such as mixed precision.

> [!IMPORTANT]
> If a new feature is added, a new test should be added to cover it.
> The minimum overall test coverage for a PR to be merged is 90%.
Expand All @@ -136,6 +141,24 @@ Our GitHub Actions workflow is run when a new PR is opened.
1. Next, the [type checking](#type-checking) is run.
1. If all previous checks pass, the [tests](#tests) are run.

The tests run in five suites, one job each.
The GPU runner is billed by the minute, so only the suites that need a GPU run there.
The GPU suites start only after the `cpu` suite passes, so that a failure it finds costs no time on the GPU runner:

| Suite | Tests | Runner |
| -------------------------------------- | ------------------------------------------------------------------ | --------------- |
| `cpu` | The unit tests, and the integration tests without the `gpu` marker | Free CPU runner |
| `misc` | The integration tests with the `gpu` marker | GPU runner |
| `predefined_light`, `predefined_heavy` | `test_predefined_models.py`, the light and the heavy variants | GPU runner |
| `combinations` | `test_combinations.py` | GPU runner |

Two rules keep the GPU suites from running when they cannot find anything new:

- A pull request that changes only docstrings, comments, Markdown files or the settings of the linters and of the API docs runs the `cpu` suite only.
`scripts/only_docstrings_changed.py` decides this.
- A draft pull request skips `predefined_heavy`.
The suite runs when you mark the pull request as ready for review.

> [!TIP]
> Review the GitHub Actions output if your PR fails.

Expand Down
118 changes: 118 additions & 0 deletions scripts/only_docstrings_changed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
"""Tell whether the changes since a base commit touch only
documentation.

The CI runs the GPU test suites only when the code changes. The script
exits with 0 when each changed file is one of these:

- a Python file whose syntax tree without docstrings stays the same.
Comments and formatting never reach the syntax tree;
- C{pyproject.toml} with changes in the tables of the linters and the
API docs only, see C{DOC_TABLES};
- a Markdown file, C{.gitignore} or C{.pre-commit-config.yaml}.

Any other change, an added or a removed Python file included, exits
with 1 and names the file.

Usage: C{python scripts/only_docstrings_changed.py <base-commit>}

"""

import ast
import subprocess
import sys
from collections.abc import Callable

DOC_TABLES = (
"tool.docformatter",
"tool.pydoctor",
"tool.pyright",
"tool.ruff",
)
IGNORED = {".gitignore", ".pre-commit-config.yaml"}


def git(*args: str) -> str:
"""Run git and return its output."""
return subprocess.run(
["git", *args], # noqa: S607
check=True,
capture_output=True,
text=True,
).stdout


def is_bare_string(statement: ast.stmt) -> bool:
"""Tell whether a statement is a string literal and nothing else."""
return (
isinstance(statement, ast.Expr)
and isinstance(statement.value, ast.Constant)
and isinstance(statement.value.value, str)
)


def code_of(source: str) -> str:
"""Dump the syntax tree of C{source} without its docstrings.

A string literal that stands as a statement of its own has no
effect, wherever it is. This covers the docstrings of attributes
too.

"""
tree = ast.parse(source)
for node in ast.walk(tree):
for field in ("body", "orelse", "finalbody"):
statements = getattr(node, field, None)
if isinstance(statements, list):
setattr(
node,
field,
[s for s in statements if not is_bare_string(s)],
)
return ast.dump(tree)


def without_doc_tables(pyproject: str) -> list[str]:
"""Drop the tables of C{DOC_TABLES} from C{pyproject.toml}.

A table header always stands on a line of its own, at the start.

"""
kept = []
keep = True
for line in pyproject.splitlines():
if line.startswith("["):
keep = not line.strip("[] ").startswith(DOC_TABLES)
if keep:
kept.append(line)
return kept


def main(base: str) -> int:
merge_base = git("merge-base", base, "HEAD").strip()
changes = git("diff", "--name-status", "--no-renames", merge_base, "HEAD")
for line in changes.splitlines():
status, path = line.split("\t")
if path.endswith(".md") or path in IGNORED:
continue
compare: Callable[[str], object]
if path == "pyproject.toml":
compare = without_doc_tables
elif path.endswith(".py"):
compare = code_of
else:
print(f"{path}: not a documentation file")
return 1
if status != "M":
print(f"{path}: added or removed")
return 1
before = git("show", f"{merge_base}:{path}")
after = git("show", f"HEAD:{path}")
if compare(before) != compare(after):
print(f"{path}: the code changed")
return 1
print("Only the documentation changed.")
return 0


if __name__ == "__main__":
sys.exit(main(sys.argv[1]))
26 changes: 24 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,10 +427,20 @@ def current_opset() -> int:


def pytest_collection_modifyitems(items: list[Function]):
"""Sort the tests into the CI suites, one marker for each suite.

The suite of an integration test outside the model suites depends on
the ``gpu`` marker. A test that trains, tunes or quantizes a model
carries it and goes to ``misc``, which runs on the GPU runner. Every
other test goes to ``cpu``, together with the unit tests, and runs
on a free runner.

"""
for item in items:
path = str(item.fspath)
if "/unittests/" in path:
item.add_marker(pytest.mark.unit)
item.add_marker(pytest.mark.cpu)
item.add_marker(pytest.mark.order(0))
elif "test_predefined_models.py" in path:
item.add_marker(pytest.mark.predefined)
Expand All @@ -441,13 +451,24 @@ def pytest_collection_modifyitems(items: list[Function]):
elif "test_overfit_convergence.py" in path:
item.add_marker(pytest.mark.overfit_convergence)
item.add_marker(pytest.mark.order(4))
else:
elif item.get_closest_marker("gpu"):
item.add_marker(pytest.mark.misc)
item.add_marker(pytest.mark.order(1))
else:
item.add_marker(pytest.mark.cpu)
item.add_marker(pytest.mark.order(1))


def pytest_configure(config: Config):
config.addinivalue_line("markers", "unit: mark test as a unit test")
config.addinivalue_line(
"markers",
"gpu: the test trains, tunes or quantizes a model, "
"so it belongs to the misc suite on the GPU runner",
)
config.addinivalue_line(
"markers", "cpu: mark test as a test of the CPU suite"
)
config.addinivalue_line(
"markers", "predefined: mark test as a predefined model test"
)
Expand All @@ -463,7 +484,8 @@ def pytest_configure(config: Config):
"markers", "combinations: mark test as a combinations test"
)
config.addinivalue_line(
"markers", "misc: mark test as a miscellaneous test"
"markers",
"misc: mark test as a training test outside the model suites",
)
config.addinivalue_line(
"markers",
Expand Down
3 changes: 2 additions & 1 deletion tests/files/custom_archive_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
],
"reverse_channels": null,
"interleaved_to_planar": null,
"dai_type": "RGB888p"
"dai_type": "RGB888p",
"resize_mode": "LETTERBOX"
}
}
],
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/test_callbacks.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import json
from pathlib import Path

import pytest
import torch
from luxonis_ml.data import LuxonisDataset
from luxonis_ml.typing import Params

from luxonis_train.config import Config
from luxonis_train.core import LuxonisModel

pytestmark = pytest.mark.gpu


def test_callbacks(coco_dataset: LuxonisDataset, opts: Params, save_dir: Path):
config_file = "luxonis_train/configs/segmentation_light_model.yaml"
Expand Down
1 change: 1 addition & 0 deletions tests/integration/test_cli_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def skip_if_no_aimet() -> None:
pytest.skip("AIMET is not installed")


@pytest.mark.gpu
def test_cli_command_success(
coco_dataset: LuxonisDataset,
tmp_path: Path,
Expand Down
29 changes: 26 additions & 3 deletions tests/integration/test_combinations.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,30 @@ def test_combinations(
with subtests.test("export"):
model.export()

with subtests.test("archive"):
model.archive()

model.finalize_run()


def test_combinations_archive(
parking_lot_dataset: LuxonisDataset, opts: Params
):
"""Archive the model of `test_combinations` with one backbone.

The archive used to run for every backbone, and it took two thirds
of the time of the suite: it compresses the ONNX file of a model
with eight heads with xz, which is slow. The backbone does not
change what the archive tests. The archive reads the inputs and
the outputs from the ONNX file, and the head configs from the
heads. The backbone changes only the output shapes, not the code
that runs. The export, which the backbone does change, still runs
for every backbone in `test_combinations`.

EfficientRep is the backbone of the predefined detection models.
The test does not train, because the archive does not depend on
the weights.

"""
opts |= {
"loader.params.dataset_name": parking_lot_dataset.identifier,
}
model = LuxonisModel(get_config("EfficientRep"), opts)
assert model.archive().exists()
Loading
Loading