Skip to content
Merged
72 changes: 67 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ on:
description: "HubAI API key"
required: true
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
paths:
- ".python-version"
- ".gitignore"
- "**/*.md"
- "pyproject.toml"
- "uv.lock"
- "requirements*.txt"
Expand All @@ -63,7 +66,6 @@ on:
- "luxonis_train/**"
- "tests/**"
- ".github/workflows/ci.yaml"
- "!**/*.md"

concurrency:
# External callers can provide an isolated concurrency group.
Expand All @@ -74,6 +76,42 @@ permissions:
contents: read

jobs:
# Selects the GPU suites. A change of the documentation only runs none
# (see scripts/only_docstrings_changed.py), and a draft skips the heavy one.
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.event.pull_request.head.sha }}
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 +324,18 @@ jobs:

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

# The suites come from `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:
steps: &test-steps
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
with:
Expand Down Expand Up @@ -363,10 +403,32 @@ jobs:
if-no-files-found: ignore
overwrite: true

# Waits for the `cpu` suite, so that its failures cost no GPU time.
gpu-tests:
# GitHub appends the suite. A skipped job then shows "tests".
name: tests
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 only uploads nothing.
coverage:
needs: tests
needs:
- plan
- tests
- gpu-tests
if: ${{ needs.plan.outputs.code_changed == 'true' }}
runs-on: ubuntu-latest

permissions:
Expand Down
24 changes: 24 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 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,25 @@ 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 suites start only after the `cpu` suite passes, so that a failure there costs no GPU time:

| Suite | Tests | Runner |
| -------------------------------------- | ------------------------------------------------------------------ | ----------------- |
| `cpu` | The unit tests, and the integration tests without the `gpu` marker | `ubuntu-latest` |
| `misc` | The integration tests with the `gpu` marker | `ubuntu-t4-4core` |
| `predefined_light`, `predefined_heavy` | `test_predefined_models.py`, the light and the heavy variants | `ubuntu-t4-4core` |
| `combinations` | `test_combinations.py` | `ubuntu-t4-4core` |

CI does not run `test_overfit_convergence.py`, which has its own `overfit_convergence` marker.

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
103 changes: 103 additions & 0 deletions scripts/only_docstrings_changed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""Tell whether a branch changes only its documentation.

Exit with 0 when each changed file is a Markdown file, ``.gitignore``,
``.pre-commit-config.yaml``, ``pyproject.toml`` with changes in
``DOC_TABLES`` only, or a Python file whose syntax tree without
docstrings stays the same. Otherwise exit with 1 and name the file.

Usage: ``python scripts/only_docstrings_changed.py <base-commit>``

"""

import ast
import subprocess
import sys

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 ``source`` without its docstrings.

A string literal that stands as a statement has no effect, so the
docstrings of attributes go 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 ``DOC_TABLES`` from ``pyproject.toml``."""
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
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]))
25 changes: 22 additions & 3 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,27 +427,45 @@ def current_opset() -> int:


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

An integration test outside the model suites goes to ``misc`` when
it has the ``gpu`` marker, and to ``cpu`` otherwise.

"""
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)
item.add_marker(pytest.mark.order(2))
elif "test_combinations.py" in path:
elif (
"test_combinations.py" in path
and item.name != "test_combinations_archive"
):
item.add_marker(pytest.mark.combinations)
item.add_marker(pytest.mark.order(3))
elif "test_overfit_convergence.py" in path:
item.add_marker(pytest.mark.overfit_convergence)
item.add_marker(pytest.mark.order(4))
else:
item.add_marker(pytest.mark.misc)
gpu = item.get_closest_marker("gpu")
item.add_marker(pytest.mark.misc if gpu else 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: mark test as a test that trains, tunes or quantizes a model",
)
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 +481,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
25 changes: 22 additions & 3 deletions tests/integration/test_combinations.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,26 @@ 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,
request: pytest.FixtureRequest,
):
"""Archive the model of `test_combinations` once.

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, so one backbone is enough. `test_combinations`
exports every backbone. The archive does not depend on the weights,
so the test does not train.

"""
assert request.node.get_closest_marker("cpu") is not None
opts |= {
"loader.params.dataset_name": parking_lot_dataset.identifier,
}
model = LuxonisModel(get_config("EfficientRep"), opts)
assert model.archive().exists()
Loading
Loading