Repository navigation
chore: post-rename + post-vulkan-removal sweep (clang-rt-19, vmaf-tune choices, docs) - #511
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR is a post-rename / post-Vulkan-removal cleanup sweep that (1) fixes dev-container sanitizer builds against clang-19, (2) removes the deprecated vulkan score backend option from vmaf-tune per ADR-0726, and (3) updates CI/docs/tests to match the current backend and path conventions.
Changes:
- Add
libclang-rt-19-devto the dev container build deps so clang-19 ASan/UBSan Meson builds link correctly. - Remove
vulkanfromvmaf-tunescore-backend selection (ALL_BACKENDS, fallback chain, probing) and update unit/regression tests accordingly. - Update user docs, agent invariants, and Go CI Meson invocation to reflect the new backend set and
core/source-dir layout.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/vmaf-tune/src/vmaftune/score_backend.py | Removes Vulkan from backend enum/fallbacks and deletes Vulkan probing; updates selection semantics to reject vulkan. |
| tools/vmaf-tune/tests/test_score_backend.py | Updates unit tests for backend parsing/selection/argv wiring after Vulkan removal; adds ValueError expectations. |
| tools/vmaf-tune/tests/test_ladder_score_backend_0511.py | Updates ladder CLI regression expectations for --score-backend choices (adds hip, removes vulkan). |
| tools/vmaf-tune/tests/test_backend_precheck_0613.py | Adjusts ADR-0613 precheck regression tests to use a non-Vulkan backend (sycl) for “known-but-unavailable” behavior. |
| tools/vmaf-tune/AGENTS.md | Updates invariant notes documenting strict backend selection and the supported backend enum post-ADR-0726. |
| docs/usage/vmaf-tune-score-backend.md | Updates accepted values, fallback chain, and ADR references; adds note that Vulkan is removed. |
| docs/rebase-notes.md | Notes that the changes are post-rename cleanup with no upstream rebase impact. |
| dev/Containerfile | Adds libclang-rt-19-dev to fix clang-19 sanitizer runtime linking in the container. |
| changelog.d/fixed/containerfile-post-rename-sweep.md | Changelog entry capturing the container, backend, docs, and CI fixes. |
| .github/workflows/go-ci.yml | Fixes Meson invocation to pass core as the source directory post-rename. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+7
| `vmaf` exposes a unified ``--backend NAME`` selector | ||
| (values: ``auto|cpu|cuda|sycl|vulkan|hip``) per ADR-0127 / ADR-0175 / | ||
| ADR-0422. | ||
| (values: ``auto|cpu|cuda|sycl|hip``) per ADR-0127 / ADR-0175 / | ||
| ADR-0422. The Vulkan backend was removed in ADR-0726. |
Comment on lines
60
to
+62
| #: Backends the vmaf CLI accepts via ``--backend NAME``. | ||
| ALL_BACKENDS: tuple[str, ...] = ("cpu", "cuda", "sycl", "hip", "vulkan") | ||
| #: Vulkan was removed in ADR-0726; the enum no longer includes it. | ||
| ALL_BACKENDS: tuple[str, ...] = ("cpu", "cuda", "sycl", "hip") |
Comment on lines
257
to
259
| - ``prefer="auto"`` walks ``fallbacks`` and returns the first | ||
| entry present in ``available``. ``cpu`` must be in the chain | ||
| (or in ``available``) to guarantee a result. |
Comment on lines
+34
to
+38
| | `auto` | (default) probe the host and pick the fastest available, in the order `cuda → sycl → hip → cpu`. | | ||
| | `cuda` | NVIDIA GPU score path. Errors out if the local `vmaf` was built without CUDA, or if `nvidia-smi` is missing / reports no devices. | | ||
| | `sycl` | Intel oneAPI SYCL path. Errors out if `vmaf` was built without SYCL or `sycl-ls` reports no devices. | | ||
| | `hip` | AMD ROCm score path. Errors out if the local `vmaf` was built without HIP, or if `rocminfo` / `rocm-smi` cannot see an AMD GPU. | | ||
| | `cpu` | Force CPU scoring. Always available; useful as a baseline reference when chasing a numeric divergence. | |
This was referenced Jun 1, 2026
lusoris
marked this pull request as draft
June 1, 2026 14:57
auto-merge was automatically disabled
June 1, 2026 14:57
Pull request was converted to draft
…une choices, docs Four post-rename / post-Vulkan-removal stragglers surfaced by the 2026-06-01 container audit: 1. dev/Containerfile: add libclang-rt-19-dev next to clang-19/clang++-19 so that `b_sanitize=address,undefined` meson builds link against the clang-19 ASan/UBSan runtime (was failing: cannot find libclang_rt.asan.a). 2. vmaf-tune score_backend: remove vulkan from ALL_BACKENDS, DEFAULT_FALLBACKS, _probe_vulkan, and the detect_available_backends probes table (ADR-0726). Requesting --score-backend vulkan now raises ValueError at startup. 3. vmaf-tune tests: update test_score_backend.py, test_ladder_score_backend_0511.py, and test_backend_precheck_0613.py to reflect the removal; replace vulkan-specific test bodies with hip equivalents where appropriate. 4. docs: update docs/usage/vmaf-tune-score-backend.md (accepted-values table, fallback chain, see-also) and tools/vmaf-tune/AGENTS.md invariant note. Note: dev/Containerfile COPY/cd paths already use core/ (ADR-0700 was previously applied to the load-bearing lines); the only remaining libvmaf/ reference at line 459 is an intentional historical comment documenting the rename. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sanitizer shadow)
lusoris
force-pushed
the
fix/post-rename-post-vulkan-sweep
branch
from
June 2, 2026 15:08
4d11f27 to
4965510
Compare
lusoris
marked this pull request as ready for review
June 2, 2026 15:08
lusoris
added a commit
that referenced
this pull request
Sep 5, 2026
The first version of this gate anchored row ids on `^\| \*\*(T-[A-Z0-9-]+)\*\* \|`,
which only matches a row whose first cell is the bold id and nothing else. The
majority shape in docs/state.md is `| **T-ID** -- one-line description | ...`,
so 30 of 335 rows were invisible to the gate, and its character class excluded
`.`, hiding every dotted id.
Two real duplicate pairs survived the first sweep because of exactly those two
holes, and the gate reported OK over them:
- T-SYCL-DMABUF-IMPORT-WIN32-ENOSYS was filed once under "Confirmed
not-affected (by design)" and again under "Deferred (waiting on external
trigger)" while its own text says "no action required". Kept the
not-affected row, which is the accurate verdict, and folded in the
ADR-0374 disabled-interface citation the deferred row carried.
- T-VK-VIF-1.4-RESIDUAL was filed twice under "Recently closed". Kept the
verified row citing PR #511 (merged 2026-05-09) and dropped the earlier
one, whose provenance cell still read "this PR" -- a placeholder the
ADR-0165 touch gate rejects.
The matcher now anchors on the bold token that opens the first cell and admits
`.` in ids. Two regression cases cover the shapes that slipped through; both
are detected by the new matcher and invisible to the old one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
that referenced
this pull request
Sep 5, 2026
…hat way (#1322) * fix(docs): one row per bug id in state.md, and a gate that keeps it that way Five bug ids appeared twice. T-CUDA-INIT-SUBMIT-LEAKS-2026-06-19 and T-UPSTREAM-1564-ADM-CM-GPU-BORDER-AND-ROUNDING-2026-09-03 sat in BOTH "Open bugs" (the stale present-tense row) and "Recently closed" (the past-tense row the fixing PR added), so two bugs that are fixed on master read as open in every audit and in every "what is left for 1.0.0" count. Three more rows were duplicated inside "Recently closed". The cause is the keep-both conflict resolution that the merge train and the agent worktrees use for the append-only doc sections: it is correct when two PRs each append a row, and wrong when one PR is MOVING a row between sections. scripts/ci/check-state-md-rows.sh now rejects any id that appears more than once, wired into pre-commit, `make lint-sh` and the Rules workflow, with a hermetic self-test covering both shapes (same id in two sections, same id twice in one section) and the clean/missing-file cases. * fix(ci): match every state.md row shape, not just bare-id first cells The first version of this gate anchored row ids on `^\| \*\*(T-[A-Z0-9-]+)\*\* \|`, which only matches a row whose first cell is the bold id and nothing else. The majority shape in docs/state.md is `| **T-ID** -- one-line description | ...`, so 30 of 335 rows were invisible to the gate, and its character class excluded `.`, hiding every dotted id. Two real duplicate pairs survived the first sweep because of exactly those two holes, and the gate reported OK over them: - T-SYCL-DMABUF-IMPORT-WIN32-ENOSYS was filed once under "Confirmed not-affected (by design)" and again under "Deferred (waiting on external trigger)" while its own text says "no action required". Kept the not-affected row, which is the accurate verdict, and folded in the ADR-0374 disabled-interface citation the deferred row carried. - T-VK-VIF-1.4-RESIDUAL was filed twice under "Recently closed". Kept the verified row citing PR #511 (merged 2026-05-09) and dropped the earlier one, whose provenance cell still read "this PR" -- a placeholder the ADR-0165 touch gate rejects. The matcher now anchors on the bold token that opens the first cell and admits `.` in ids. Two regression cases cover the shapes that slipped through; both are detected by the new matcher and invisible to the old one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
libclang-rt-19-devto the build-deps apt layer sob_sanitize=address,undefinedmeson builds link against the clang-19 ASan/UBSan runtime (cannot find libclang_rt.asan.awas failing)--score-backend: removevulkanfromALL_BACKENDS,DEFAULT_FALLBACKS, and all argparsechoices=tuples following ADR-0726 (Vulkan backend removal); requesting--score-backend vulkannow raisesValueErrorat startuptest_score_backend.py,test_ladder_score_backend_0511.py,test_backend_precheck_0613.pyto reflect the removal; replace vulkan-specific test bodies with hip equivalentsdocs/usage/vmaf-tune-score-backend.md(accepted-values table, fallback chaincuda→sycl→hip→cpu, see-also) andtools/vmaf-tune/AGENTS.mdinvariant noteNote: the Containerfile's load-bearing
COPY/cdpaths already usecore/(ADR-0700 was previously applied). The only remaininglibvmaf/reference is at line 459 — an intentional historical comment documenting the rename that should be preserved.Reproducer / smoke-test
ADR-0108 deliverables checklist
tools/vmaf-tune/AGENTS.mdto reflectcpu|cuda|sycl|hipenum and ADR-0726 notechangelog.d/fixed/containerfile-post-rename-sweep.mddocs/rebase-notes.md)no docs needed: SYCL integer extractor forward-decl is a pure compile fix (master regression from ADR-0784 partial-init cleanup paths) — no user-visible behavior change, no API delta. Per CLAUDE.md §12 r10: "Internal refactors, bug fixes with no user-visible delta, and test-only changes are excluded."