-
Notifications
You must be signed in to change notification settings - Fork 17
[Misc] Add coverage report to PRs, including kernels #470
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
Changes from all commits
773110d
05732ca
bd993fa
ac86607
580738b
331b31a
cbe3c72
dd0e850
5e13cee
4406489
caa3ee5
4427f13
8c5e615
b2ec13c
d47004c
fd27142
565da3d
eadc831
77c3da4
795b3b5
0c4cc67
b480a06
470fb92
68c39fb
98a17d5
d9482bb
92f00d5
72f01d8
e83ed6c
c5d4a18
3f7b30f
54a3293
c02bace
46acd3a
5940f9d
81c1a08
0409574
5016556
fe67ca0
ed4ef06
382684f
e53aafb
b7f9770
bb3004b
59d8a59
46fb3fe
6fbf61a
73958db
2ea9d35
fa8377c
b671ed6
6a6d61e
7ef7449
1ab9a9c
948ce37
205e54b
6327054
ce6c367
97e5b8f
c811390
d45071c
c855602
44c5b11
f2b9c87
58bfaad
1efd3bc
bc51d51
16debd6
6ac266c
9a433e7
4a8a670
24d608c
4f3254d
54db562
683ff9b
42c88da
52decab
4961717
b187b37
6221120
c68e312
e8c4200
fe0222f
e8cbf37
ceb6805
c968085
cb088dc
f8fb179
436d992
ca918ee
7c2e1b3
d1ce0bf
890f9dd
82243d8
3f305d4
c79b862
e89b53b
392c1ba
5d20c67
17a467d
2d4d966
76678c6
4d0e423
6315287
03aac48
08637e1
b7c6ead
9237abe
570aa65
f773b39
f37d280
c3a8c75
baffc00
cbe01ec
2ef75b5
b38b311
932bd9a
e951652
01150ba
76f2f53
ccf2dad
79f29b5
edd8e92
dbef8ad
8bc367e
ef2f2bd
484089d
2c21f36
572fcc8
352b029
d6d50e2
d3e4bad
50b711d
245de20
b80cb55
f4052b1
3c3191a
920b379
fa460fe
786e1dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,8 @@ on: | |
| - reopened | ||
| - synchronize | ||
| workflow_dispatch: | ||
| permissions: | ||
| contents: read | ||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} | ||
| cancel-in-progress: true | ||
|
|
@@ -21,6 +23,8 @@ jobs: | |
| runs-on: ${{ matrix.OS }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we changing this? |
||
| - name: Python check | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
|
|
@@ -37,3 +41,109 @@ jobs: | |
| - name: Linux test | ||
| run: | | ||
| bash .github/workflows/scripts_new/linux/4_test.sh | ||
| - name: Upload wheel for CUDA job | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: linux-wheel | ||
| path: dist/*.whl | ||
| - name: Upload CPU coverage data | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: coverage-cpu | ||
| path: | | ||
| coverage.xml | ||
| pytest-coverage.txt | ||
|
|
||
| test-cuda: | ||
| name: Linux CUDA Test | ||
| needs: build | ||
| runs-on: gpu-t4-4-core | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Python check | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.10' | ||
| - name: Install CUDA libraries | ||
| run: | | ||
| sudo apt-get install -y libcusolver-dev-12-8 libcusolver-12-8 libcusparse-dev-12-8 libcusparse-12-8 libnvjitlink-12-8 libcublas-12-8 | ||
| echo "/usr/local/cuda/targets/x86_64-linux/lib" | sudo tee /etc/ld.so.conf.d/cuda-targets.conf | ||
| sudo ldconfig | ||
| - name: Download wheel | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: linux-wheel | ||
| - name: Install quadrants | ||
| run: | | ||
| set -x | ||
| mkdir -p dist | ||
| mv *.whl dist/ | ||
| pip install dist/*.whl | ||
| - name: Install test requirements | ||
| run: | | ||
| pip install --group test | ||
| pip install -r requirements_test_xdist.txt | ||
| - name: Run CUDA tests with coverage | ||
| run: | | ||
| bash .github/workflows/scripts_new/linux/4_test_cuda.sh | ||
| - name: Upload CUDA coverage data | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: coverage-cuda | ||
|
claude[bot] marked this conversation as resolved.
|
||
| path: coverage.xml | ||
|
|
||
| coverage-comment: | ||
|
claude[bot] marked this conversation as resolved.
|
||
| if: github.event_name == 'pull_request' && always() | ||
| needs: [build, test-cuda] | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Python check | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.10' | ||
| - name: Download CPU coverage | ||
| uses: actions/download-artifact@v4 | ||
| continue-on-error: true | ||
| with: | ||
| name: coverage-cpu | ||
| path: coverage-cpu | ||
| - name: Download CUDA coverage | ||
| uses: actions/download-artifact@v4 | ||
| continue-on-error: true | ||
|
claude[bot] marked this conversation as resolved.
|
||
| with: | ||
| name: coverage-cuda | ||
| path: coverage-cuda | ||
| - name: Generate coverage report | ||
| continue-on-error: true | ||
| run: | | ||
| COV_XMLS="" | ||
| if [ -f coverage-cpu/coverage.xml ]; then | ||
| COV_XMLS="coverage-cpu/coverage.xml" | ||
| fi | ||
| if [ -f coverage-cuda/coverage.xml ]; then | ||
| COV_XMLS="$COV_XMLS coverage-cuda/coverage.xml" | ||
| fi | ||
| if [ -z "$COV_XMLS" ]; then | ||
| echo "No coverage XML files found, skipping report" | ||
| exit 0 | ||
| fi | ||
|
|
||
| python tests/coverage_report.py --report-only \ | ||
| --compare-branch=origin/${{ github.base_ref }} \ | ||
| --coverage-xml $COV_XMLS \ | ||
| --format markdown > coverage-comment.md | ||
| - name: Post coverage comment | ||
| if: always() && hashFiles('coverage-comment.md') != '' | ||
| run: gh pr comment ${{ github.event.pull_request.number }} --body-file coverage-comment.md | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
|
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -ex | ||
|
|
||
| TEST_EXIT=0 | ||
|
|
||
| # Disable kernel-level coverage on CUDA: it changes field memory layout and breaks dlpack tests | ||
| # (ValueError: Expected zero byte_offset). Python code coverage (--cov) still runs. | ||
| QD_KERNEL_COVERAGE=0 python tests/run_tests.py -v -r 1 --arch cuda --coverage -m "not needs_torch" || TEST_EXIT=$? | ||
|
|
||
| pip install torch --index-url https://download.pytorch.org/whl/cu128 | ||
| QD_KERNEL_COVERAGE=0 python tests/run_tests.py -v -r 1 --arch cuda --coverage --cov-append -m needs_torch || TEST_EXIT=$? | ||
|
claude[bot] marked this conversation as resolved.
|
||
|
|
||
| # Run kernel coverage tests on CUDA with coverage enabled — these are skipped by the phases above | ||
| # (QD_KERNEL_COVERAGE=0) and include GPU-only tests like test_kernel_coverage_simt_e2e. | ||
| QD_KERNEL_COVERAGE=1 python tests/run_tests.py -v -r 1 --arch cuda --coverage --cov-append test_kernel_coverage.py || TEST_EXIT=$? | ||
|
|
||
| python tests/coverage_report.py --collect-only | ||
|
|
||
| exit $TEST_EXIT | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,8 +65,13 @@ __pycache__ | |
| /python/test_env | ||
| /CHANGELOG.md | ||
| /.coverage | ||
| /.coverage.* | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| _qd_kcov.* | ||
| /coverage.xml | ||
| /coverage-report.html | ||
| /htmlcov | ||
| /diff-cover.* | ||
| /pytest-coverage.txt | ||
| libpython_path.txt | ||
| .vscode | ||
| _build | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| # Kernel code coverage | ||
|
|
||
| Standard Python coverage tools only measure host-side code. Quadrants kernel coverage goes further — it tracks which lines actually execute *inside* compiled kernels on the device (CPU or GPU), including which branches of `if`/`else` blocks are taken at runtime. | ||
|
|
||
| The coverage data is written in the standard `coverage.py` format, so it works with `coverage report`, `pytest-cov`, `diff-cover`, and IDE coverage viewers out of the box. | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| Kernel coverage requires the `coverage` Python package: | ||
|
|
||
| ```bash | ||
| pip install coverage | ||
| ``` | ||
|
|
||
| ## Enabling kernel coverage | ||
|
|
||
| ### Automatic with pytest-cov | ||
|
|
||
| If you use `pytest-cov`, kernel coverage is enabled automatically — no configuration needed. Quadrants ships a pytest plugin that detects `--cov` and sets `QD_KERNEL_COVERAGE=1` for you. Just run: | ||
|
|
||
| ```bash | ||
| pytest --cov=my_package --cov-branch tests/ | ||
| ``` | ||
|
|
||
| To disable kernel coverage while still collecting Python coverage, opt out explicitly: | ||
|
|
||
| ```bash | ||
| QD_KERNEL_COVERAGE=0 pytest --cov=my_package --cov-branch tests/ | ||
| ``` | ||
|
|
||
| ### Manual with any script | ||
|
|
||
| For scripts outside pytest, set the `QD_KERNEL_COVERAGE` environment variable: | ||
|
|
||
| ```bash | ||
| QD_KERNEL_COVERAGE=1 python my_simulation.py | ||
| ``` | ||
|
|
||
| This works with any script that uses quadrants kernels — no changes to your code are needed. | ||
|
|
||
| When the process exits, quadrants writes one or more `_qd_kcov.<pid>` files in the working directory containing the collected coverage data. | ||
|
|
||
| ## Viewing results | ||
|
|
||
| ### With coverage.py | ||
|
|
||
| Combine the kernel coverage files and produce a report using the standard `coverage` tool: | ||
|
|
||
| ```bash | ||
| # Combine all kernel coverage files into .coverage | ||
| coverage combine _qd_kcov.* | ||
|
|
||
| # Terminal summary | ||
| coverage report --show-missing | ||
|
|
||
| # HTML report | ||
| coverage html | ||
| ``` | ||
|
|
||
| ### With pytest-cov | ||
|
|
||
| When using `pytest-cov`, kernel coverage is enabled automatically (see above). The kernel coverage data is merged with Python coverage after the run: | ||
|
|
||
| ```bash | ||
| coverage combine _qd_kcov.* .coverage | ||
| ``` | ||
|
|
||
| ## Key properties | ||
|
|
||
| - **Zero overhead when disabled.** The coverage module is never imported unless `QD_KERNEL_COVERAGE=1` is set. There is no cost in normal operation. | ||
| - **Branch coverage.** Probes inside `if`/`else` bodies only fire when that branch is taken, giving true runtime branch coverage — not just kernel-level coverage, or static conditional coverage. | ||
| - **Works with pytest-xdist.** Each worker writes to a separate file; combine them afterward. | ||
| - **Survives `qd.init()` resets.** Coverage data is accumulated across multiple `qd.init()` calls within the same process. | ||
|
|
||
| ## Advanced usage | ||
|
|
||
| ### Probe capacity | ||
|
|
||
| There is a limit of 100,000 coverage probes per process (one probe per unique source line per kernel/func). If you hit the limit — for example in a very large codebase with many kernels — increase it via the environment variable: | ||
|
|
||
| ```bash | ||
| QD_COVERAGE_MAX_PROBES=500000 QD_KERNEL_COVERAGE=1 python my_simulation.py | ||
| ``` | ||
|
|
||
| ## Coverage and autodiff | ||
|
|
||
| The forward pass is covered. The backward pass is not, because instrumenting it would interfere with gradient computation. This is normally fine — the backward pass is auto-generated and replays the same control flow, so forward coverage is sufficient. | ||
|
|
||
| One edge case: kernel calls inside a `qd.ad.Tape` with `validation=True` will not be covered. | ||
|
|
||
| ## Offline cache interaction | ||
|
|
||
| Coverage probes change the compiled kernel, so the offline cache will see them as new kernels and recompile. This is expected and does not affect correctness, but the first run with coverage enabled will be slower if you normally rely on cached kernels. | ||
|
|
||
| ## CI integration | ||
|
|
||
| The CI workflow posts a diff coverage report as a PR comment on each push. A **new comment** is created each time (rather than editing the previous one) so that the PR timeline shows a clear chronological sequence of commits and their corresponding coverage results. | ||
|
|
||
| ## Under the hood | ||
|
|
||
| When `QD_KERNEL_COVERAGE=1` is set, quadrants rewrites the Python AST of each `@qd.kernel` and `@qd.func` before compilation. It inserts lightweight probe statements (`field[probe_id] = 1`) at each source line. These probes compile as ordinary field stores and execute on the device alongside your kernel code. | ||
|
|
||
| At process exit, the probe data is read back from the device and written to a `.coverage`-compatible file. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| import ast | ||
| import inspect | ||
| import math | ||
| import os | ||
| import sys | ||
| import textwrap | ||
| import types | ||
|
|
@@ -21,6 +22,11 @@ | |
|
|
||
| import numpy as np | ||
|
|
||
|
|
||
| def _kernel_coverage_enabled() -> bool: | ||
| return os.environ.get("QD_KERNEL_COVERAGE") == "1" | ||
|
|
||
|
|
||
| from quadrants._lib import core as _qd_core | ||
| from quadrants._lib.core.quadrants_python import KernelLaunchContext | ||
| from quadrants.lang import _kernel_impl_dataclass, impl | ||
|
|
@@ -246,9 +252,21 @@ def get_tree_and_ctx( | |
|
|
||
| autodiff_mode = current_kernel.autodiff_mode | ||
|
|
||
| _kcov = None | ||
| if _kernel_coverage_enabled() and autodiff_mode == _qd_core.AutodiffMode.NONE: | ||
| from . import ( # pylint: disable=import-outside-toplevel | ||
| _kernel_coverage as _kcov, | ||
| ) | ||
|
|
||
| tree = _kcov.rewrite_ast(tree, function_source_info.filepath, function_source_info.start_lineno) | ||
|
|
||
| quadrants_callable = current_kernel.quadrants_callable | ||
| is_pure = quadrants_callable is not None and quadrants_callable.is_pure | ||
| global_vars = self._get_global_vars(self.func) | ||
| if _kcov is not None: | ||
| cov_field = _kcov.get_field() | ||
| if cov_field is not None: | ||
| global_vars[_kcov.FIELD_VAR_NAME] = cov_field | ||
|
Comment on lines
+255
to
+269
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 A TOCTOU race condition exists between Extended reasoning...What the bug is and how it manifests In The specific code path that triggers it
Step-by-step proof of the race
Why existing code does not prevent it The module-level Impact Any multithreaded program that compiles kernels on one thread while calling How to fix it Extend |
||
|
|
||
| template_vars = {} | ||
| if is_kernel or is_real_function: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this block can be removed now right?