Repository navigation
fix: iter6 bundle — 6 fixes (wave32 comment, UBSan, TSAN, vmaf-tune CRF, AI scripts, MCP conformance) - #865
Merged
Merged
Conversation
…(iter6-cross-backend-parity) Three iter6 cross-backend-parity findings: 1. [critical — partial] float_adm_score.hip: wave32 code was already fixed in #859 (0a9dba8); correct the lingering stale comment that still read "FADM_WARPS_PER_BLOCK = 4 (64-lane warps)" — FADM_WARPS_PER_BLOCK is now 256/32 = 8 slots (sized for the wave32 worst case). No functional change. 2. [high — already fixed] float_ssim/ssim_score.hip + integer_psnr/psnr_score.hip: wave32 runtime-warpSize fixes landed in #859 alongside float_adm; nothing further to do here. 3. [high] ssimulacra2_sycl.cpp: add Kahan (compensated-summation) state tracking to the 3-pole recursive IIR blur kernel (launch_blur<PASS>). The IIR state (prev1_k) accumulated O(eps) rounding error per step; over 4K-tall frames this exceeded the 5e-5 cross-backend parity contract vs the CPU reference. Each pole now carries a float comp_k compensation term; the standard Kahan pattern (y = candidate - comp; new_state = old_state + y; comp = (new_state - old_state) - y) bounds per-iteration error to O(eps^2) without fp64 (ADR-0220 compliant). No Netflix golden-data assertions modified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…avx512.c _mm512_set_epi64 takes long long (signed 64-bit) arguments. The literal 0xFFFFFFFFFFFFFFFF exceeds LLONG_MAX and is undefined behaviour under strict UBSan. Replace with -1LL which has identical bit pattern and correct type at both call sites (ADM_CM_THRESH_S_I_END macro lines 406 and 698). Finding: iter6-ubsan-strict high [adm_avx512: 0xFFFF... overflows long long]. Note: motion_avx512.c and adm_avx2.c analogous fixes were already present on master (PR #858 / PR #859 bundles). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er6-tsan-race-deep) Finding #1 (high): Remove the racy write `fex->framesync = vmaf->framesync` in `threaded_enqueue_one`. `fex` is the *shared* registered VmafFeatureExtractor — worker threads from previous frames may concurrently read its fields. The write is redundant: framesync is already propagated to every pool-slot copy by `set_fex_framesync()` at registration time and by `ctx_pool_ensure_slot_ctx()`. Finding #2 (high): Move the `vmaf->prev_ref` advance to BEFORE the enqueue call in `threaded_read_pictures_batch`. In the old order the main thread unreffed and replaced `vmaf->prev_ref` after enqueue while the just-submitted worker still held a live reference to the same underlying VmafRef*, creating a concurrent unref/write on the same object without synchronisation. Workers use `data.prev_ref` (an independently refcounted snapshot) exclusively and never re-read `vmaf->prev_ref` after enqueue, so moving the advance before enqueue is both safe and race-free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… strategy
Two bugs in the compare/recommend paths:
1. _encode_and_score() in bisect.py had encode_runner and score_runner as
required keyword-only args (no defaults). The CRF-sweep caller in cli.py
did not pass them, causing an unconditional TypeError on any
compare --no-bisect --crf-sweep invocation. Fixed by giving both
parameters a default of None, consistent with the existing decode_runner
parameter and the run_encode/run_score runner=None semantics.
2. _smallest_passing_crf() in cli.py used `crf > cur[0]` to select the
largest (most efficient) passing CRF, contradicting both the function
name and the CLI help string ("find the smallest CRF whose VMAF >=
--target-vmaf"). The correct strategy is the smallest (highest-quality)
passing CRF. Fixed comparison to `crf < cur[0]` and updated the docstring
to match.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… saliency parity always-fail, ensemble ONNX codec-dim mismatch
- collect_gpu_calibration_data.py: remove dead args.vulkan_device
reference from devices dict (Vulkan removed per ADR-0726; no
--vulkan-device argparse registration existed, causing AttributeError
at runtime)
- validate_saliency_student.py: replace broken PT-reconstruction parity
check with ORT-only sanity check when no PT state provided.
do_constant_folding=True folds BN stats into conv weights at export
so ONNX initializer names diverge from PT state_dict keys; the old
code silently left 60 of 65 weights at random defaults and always
failed. When pt_state is provided (trainer path) full PT<->ORT diff
is still performed.
- model/tiny/fr_regressor_v2_ensemble_v1_seed{0..4}.onnx: regenerate
with codec_onehot=[batch,6] matching current CODEC_VOCAB (was [batch,14]
from a 12-entry encoder_vocab + 2 norm dims; CODEC_VOCAB was later
trimmed to 6). Regenerated via train_fr_regressor_v2_ensemble.py
--smoke in vmaf-dev-mcp container. eval_probabilistic_proxy.py --smoke
now passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two iter6 fuzz conformance findings fixed:
1. [high] Parse-error notification instead of error response (id=null):
Add `_ParseErrorFilteredStdin` — a lazy async stdin wrapper that
pre-validates each incoming line with
`JSONRPCMessage.model_validate_json`. On failure it calls
`_emit_parse_error` which writes
`{"jsonrpc":"2.0","id":<recovered_or_null>,"error":{"code":-32700,
"message":"Parse error"}}` to stdout synchronously, then drops the
line so the mcp library never sees a bare Exception on the stream
(the notification path is bypassed entirely). `_run()` now passes
`stdin=_ParseErrorFilteredStdin()` to `stdio_server`.
2. [high] 500-level deep nesting triggers recursion-limit exception:
Add `_check_depth(obj, max_depth=50)` helper and call it at the top
of `_call_tool_dispatch` before the tool dispatch. Payloads exceeding
50 nesting levels raise `ValueError`, which the mcp library converts
to an isError=True tool result before the pydantic parser recurses.
Existing tests updated: the two `stdio_server`-patching tests in
`test_coverage_round2.py` now accept `**kwargs` so they tolerate the
new `stdin=` keyword argument forwarded by `_run()`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lusoris
added a commit
that referenced
this pull request
Jun 13, 2026
… to one-shot retrain (ADR-1105) (#902) PR #865 regenerated the five fr_regressor_v2_ensemble_v1_seed{0..4} ONNX files in --smoke mode to fix a codec_vocab 14->6 input-dim mismatch, but two side effects regressed the registry: 1. license / license_url / sigstore_bundle were dropped from the five rows, breaking test_every_entry_has_license_metadata. Restored here (every entry must carry license metadata, smoke or not). 2. The rows now ship smoke weights while ADR-0321's test asserts production (smoke=false). The ADR-0321 production weights were LOSO-validated at codec_vocab=14; the trim to 6 made them stale, and real production weights at width 6 require re-running export_ensemble_v2_seeds.py -- part of the locked one-shot post-RC retrain (ensemble is in scope; piecemeal retrain is explicitly out). For the RC: keep smoke=true (matches the shipped weights honestly), and mark test_fr_regressor_v2_ensemble_seed_rows_are_production xfail(strict=True) with an ADR-1105 reason. strict=True flips it back to a hard failure the moment the one-shot retrain lands real weights, forcing removal of the marker. No Netflix golden-data assertion modified (model_registry_schema_test.py is fork-local). - ADR-1105 (Accepted) + README index row - docs/state.md: T-ENSEMBLE-V2-PROD-FLIP-DEFERRED row - docs/ai/models/fr_regressor_v2_probabilistic.md: status block corrected - changelog.d/fixed fragment Local: python3 -m pytest python/test/model_registry_schema_test.py -> 10 passed, 1 xfailed; validate_model_registry.py -> OK 26 entries. Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
26 of 31 tasks
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
1 participant
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
Bundle of 6 iter6 validation findings. Two findings (iter6-asan-leak-deep, iter6-fuzz-extended) were already present on master in prior PRs (#859, #857) — no commits needed for those.
float_adm_score.hipstale comment corrected (FADM_WARPS_PER_BLOCK = 4 (64-lane warps)→8 (wave32 worst case)); Kahan compensated-summation added to the ssimulacra2 SYCL 3-pole IIR blur to eliminate O(eps) per-step accumulation error.0xFFFFFFFFFFFFFFFFliterals (UB — exceedsLLONG_MAX) with-1LLin two_mm512_set_epi64call sites inadm_avx512.c.fex->framesyncwrite inthreaded_enqueue_one; movevmaf->prev_refadvance to beforevmaf_thread_pool_enqueue()inthreaded_read_pictures_batchto close TSAN data race onVmafRef*._encode_and_score()defaultNoneforencode_runner/score_runner(fixesTypeErroron--no-bisect --crf-sweep); fix_smallest_passing_crf()comparison (crf > cur[0]→crf < cur[0]) so it selects the smallest (highest-quality) passing CRF as documented.args.vulkan_devicereference (AttributeError; Vulkan removed per ADR-0726); fix saliency parity check to ORT-only when no PT state supplied; regenerate ensemble ONNX models withcodec_onehot=[batch,6]matching currentCODEC_VOCAB._ParseErrorFilteredStdinpre-validator for JSON-RPC parse-error conformance (emits{"id":null,"error":{"code":-32700}}instead of notification); add_check_depth(max_depth=50)guard against recursion-limit on deeply nested payloads.Findings already resolved (no commit)
*vmaf = NULLafterfree(v)and*pool = NULLbeforefree(p)— landed in fix+chore: 30+ fixes bundle F — deferred criticals + MCP+round5 bulk #859 (0a9dba8).(size_t)cast — landed in PR fix: 12 medium/low round-3 bundle — SYCL ceiling, meson conflicts, CLI bounds, y4m, fuzz, motion debug, docs #857 (d47b8b8).Test plan
git grep '^<<<<<<'clean).Deliverables checklist
UBSAN_OPTIONS=print_stacktrace=1 ./build/tools/vmaf --no-bisect --crf-sweepfor vmaf-tune;python3 -c "import json; json.loads('{')"piped to MCP for parse-error path🤖 Generated with Claude Code