Track B: tiered fidelity contracts - #18
Conversation
`parity.policy` contracts the numeric axis, and for a fused kernel meant as a drop-in replacement `byte_equal` is the right answer. It is the wrong answer for the two families this project is moving toward: an alternative attention backend changes the accumulation order of the attention product, and a cross-step cache skips recomputing a step entirely. Neither is bit-exact, and under `byte_equal` both are rejected before they are ever benchmarked. The tempting fix is to widen the tolerance until they fit. That is the R4 failure exactly -- four VAE artifacts built on `rcp.approx.ftz.f32` reached packaging carrying up to 131072.0 of absolute error at `match=true`, because a tolerance wide enough to admit them was wide enough to admit anything. A tolerance wide enough to admit a cache hit is wider still. So the numeric axis is not widened. A second axis is added: tier 1 `exact` bitwise identity, decided by the parity policy tier 2 `perceptual` frames perceptually indistinguishable at a fixed seed tier 3 `advisory` measured and recorded, never gating, never auto-promoted A workload declaring nothing gets tier 1 -- failing closed is the point. At tier 2 the fidelity verdict *replaces* the parity check in GenerationOutcome.decide(), because a tier-2 candidate fails `byte_equal` by construction and gating on parity first would quarantine every attention and caching candidate before its evidence was read. Tier 1 behaviour is unchanged down to the wording of the quarantine reason. Thresholds are declared per workload and never invented by the harness, the same rule resolve_leaf_tolerance already enforces: a 1.3B model at 480p and a 14B at 720p do not share a perceptual noise floor. A `perceptual` budget with no threshold or no frame set is rejected at construction -- a tier with no bar is not a weaker contract, it is no contract. Evidence handling refuses to guess. Missing evidence holds the artifact; evidence from a different frame set or seed is refused rather than discounted, being evidence about a different question; a metric with no backend installed is reported absent and holds any budget gating on it. Promotions record the signed margin for every gated metric, positive always meaning passing, so a perceptual promotion can be audited from the manifest alone. The harness aggregates worst-frame rather than mean: a cache that is perfect on seven frames and destroys the eighth averages to a comfortable pass, and a single broken frame is precisely what a schedule transform produces. SSIM is computed directly from numpy so tier 2 has no optional dependency in its common path, and is cross-checked against scikit-image to ~1e-16. LPIPS loads through a pluggable backend; VBench's score is passed in from its isolated stage so nothing on the promotion path imports its model zoo. Exit criteria are covered by tests: an intentionally-lossy control artifact -- fast, cleanly dispatched, classified `improved` -- is quarantined at tier 2 on its SSIM/LPIPS alone, and a bit-inexact but perceptually clean artifact is promoted with its margins recorded.
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds tiered fidelity contracts, perceptual frame comparison, workload manifest configuration, fidelity-aware artifact promotion, persisted evidence, automated tests, and Modal verification commands. ChangesTiered fidelity verification
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WorkloadManifest
participant FinalizationAdapter
participant GenerationOutcome
participant FidelityEvaluator
participant ArtifactEvidence
WorkloadManifest->>FinalizationAdapter: provide fidelity configuration
FinalizationAdapter->>GenerationOutcome: pass budget and perceptual evidence
GenerationOutcome->>FidelityEvaluator: evaluate fidelity contract
FidelityEvaluator-->>GenerationOutcome: return verdict and metric margins
GenerationOutcome->>ArtifactEvidence: record non-exact fidelity evidence
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The tier machinery is deliberately CPU-only -- the gate must be decidable without a GPU, an image backend, or a pretrained network. That property meant the local run could not execute the parts of the suite that hard-import torch, so 21 tests were failing purely on torch's absence and 5 modules could not be collected. Those tests are not about fidelity tiers, but they are exactly the ones that would catch a tier change breaking the CLI or the built-in specs. `baseline_suite` runs the same suite at the merge-base so a failure can be shown pre-existing rather than argued to be. That distinction is the whole point: reasoning about whether a change "could plausibly" have caused a failure is not evidence, and this repository has already paid once for accepting a conclusion that was never measured.
|
@coderabbitai review |
✅ Action performedReview finished.
|
test_unknown_aliasing_fails_closed_in_every_mode[symbolic] fails on main, not because of anything in this branch: the fixture calls torch.as_strided(x, x.shape, x.stride()), and from torch 2.8 symbolic tracing hands as_strided Proxy objects where it requires concrete ints. The trace raises TypeError before any region exists, so the aliasing detector never runs and the old assertion -- region is not None -- cannot hold. Not capturing at all is strictly safer than capturing an aliasing-unsafe module, so that outcome is now accepted, but only for the mode that genuinely cannot get further, and only when capture recorded why it failed. An empty result with no recorded failure would look identical to a silent no-op and is still an error. The detector stays under the original strict assertion for export and dynamo, which is where its coverage actually lives. This narrows where the assertion applies to match where the tracer can reach; it does not lower the bar for any mode that can.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
autokernel/verification/perceptual.py (3)
190-218: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the LPIPS network, and replace the assigned lambda.
compare_frame_setscallslpips_backend()on every invocation (Line 287). Each call constructslpips_module.LPIPS(net="alex")and loads pretrained AlexNet weights. A campaign that compares many artifacts pays that load repeatedly. Cache the backend so the weights load once per process.Ruff also flags Line 214 (E731): assign a
defrather than a lambda.⚡ Proposed change
+import functools + + +@functools.lru_cache(maxsize=1) def lpips_backend() -> Callable[[Any, Any], float]:# LPIPS wants NCHW in [-1, 1]. - to_nchw = lambda a: torch.from_numpy(a).permute(0, 3, 1, 2).float() * 2 - 1 + def to_nchw(a: Any) -> Any: + return torch.from_numpy(a).permute(0, 3, 1, 2).float() * 2 - 1 + with torch.no_grad(): return float(network(to_nchw(ref), to_nchw(cand)).item())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autokernel/verification/perceptual.py` around lines 190 - 218, Update lpips_backend to cache the initialized LPIPS network/backend for reuse across calls, ensuring pretrained AlexNet weights are loaded only once per process while preserving MetricUnavailable behavior. Replace the to_nchw lambda inside _score with a local def and keep the existing tensor conversion and scoring behavior unchanged.Source: Linters/SAST tools
282-294: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAn LPIPS scorer failure escapes as an unhandled exception.
Lines 291-294 call
scorerwithout error handling. The docstring at Lines 258-261 declares onlyPerceptualError. A backend failure inside torch, for example an out-of-memory error during inference, therefore propagates out ofcompare_frame_setsunchanged.The downstream adapter catches only
FidelityErrorandTypeError(autokernel/optimize/adapters.py Lines 78-102), so such an error aborts the campaign after the GPU time is already spent. Treating a failed measurement as absent keeps the documented behavior: the budget gating on LPIPS then holds the artifact.♻️ Proposed change
if scorer is not None: - worst_lpips = max( - scorer(ref_frames[index], cand_frames[index]) - for index in range(count) - ) + try: + worst_lpips = max( + scorer(ref_frames[index], cand_frames[index]) + for index in range(count) + ) + except PerceptualError: + raise + except Exception: + # A backend that fails mid-measurement yields no number. The + # metric is absent, never substituted, so a budget gating on + # LPIPS holds the artifact. + worst_lpips = None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autokernel/verification/perceptual.py` around lines 282 - 294, Update the LPIPS scoring path in compare_frame_sets so failures while invoking scorer or aggregating its results are caught and treated as an absent measurement by leaving worst_lpips as None. Preserve the existing MetricUnavailable handling and ensure only the documented PerceptualError contract escapes from this function.
136-152: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse the separable form of the Gaussian window.
_gaussian_windowbuilds the 11×11 kernel as an outer product of a 1-D kernel, so the filter is separable._filter2dthen applies it as a dense 2-D correlation at O(k²) = 121 multiply-adds per output pixel.
ssimcalls_filter2dsix times per channel, andcompare_frame_setscallsssimonce per frame. For an 8-frame 720p RGB set that is 144 passes over roughly 0.9M pixels each, near 1.6e10 multiply-adds in NumPy. Two 1-D passes reduce this to 22 multiply-adds per pixel, about 5.5× less work, with identical results.⚡ Proposed change
-def _gaussian_window(np: Any) -> Any: +def _gaussian_window_1d(np: Any) -> Any: coords = np.arange(_WINDOW_SIZE, dtype=np.float64) - (_WINDOW_SIZE - 1) / 2.0 kernel = np.exp(-(coords**2) / (2.0 * _WINDOW_SIGMA**2)) kernel /= kernel.sum() - return np.outer(kernel, kernel) + return kernel def _filter2d(np: Any, plane: Any, window: Any) -> Any: - """Valid-mode 2-D correlation, written with strides to avoid scipy.""" + """Valid-mode 2-D correlation of a separable Gaussian, without scipy.""" height, width = plane.shape - k = window.shape[0] + k = _WINDOW_SIZE if height < k or width < k: raise PerceptualError( f"frames are {height}x{width}, smaller than the {k}x{k} SSIM window" ) - windows = np.lib.stride_tricks.sliding_window_view(plane, (k, k)) - return np.einsum("ijkl,kl->ij", windows, window) + rows = np.lib.stride_tricks.sliding_window_view(plane, k, axis=1) + horizontal = rows @ window + columns = np.lib.stride_tricks.sliding_window_view(horizontal, k, axis=0) + return columns @ window
ssimthen passes the 1-D kernel from_gaussian_window_1d.Verify the numeric equivalence against the scikit-image cross-check the documentation mentions in docs/TIERED_FIDELITY.md Line 116.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autokernel/verification/perceptual.py` around lines 136 - 152, Replace the dense 2-D Gaussian path in _gaussian_window and _filter2d with separable filtering using a normalized 1-D kernel from _gaussian_window_1d, applying horizontal and vertical correlations while preserving valid-mode dimensions and size validation. Update ssim to pass the 1-D kernel through this path, and verify numerical equivalence against the documented scikit-image cross-check.autokernel/verification/fidelity.py (1)
321-342: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer the workload spec's own
budget()accessor.
WorkloadManifest.fidelityholds aFidelitySpec(autokernel/workload/types.py Line 746), which is neither aFidelityBudgetnor aMapping. The call therefore lands in the duck-typed branch at Lines 335-342 and rebuilds the budget field by field.FidelitySpec.budget()(autokernel/workload/types.py Lines 628-639) already returns the validated budget. If a field is added toFidelitySpeclater, the duck-typed branch drops it silently.♻️ Proposed change
if isinstance(fidelity, Mapping): return cls.from_dict(fidelity) + to_budget = getattr(fidelity, "budget", None) + if callable(to_budget): + budget = to_budget() + if isinstance(budget, FidelityBudget): + return budget return cls( tier=str(getattr(fidelity, "tier", EXACT)),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autokernel/verification/fidelity.py` around lines 321 - 342, Update FidelityBudget.from_workload to detect the workload’s FidelitySpec and return its validated budget via the spec’s budget() accessor, before the field-by-field fallback branch. Preserve existing handling for FidelityBudget, mappings, and workloads without fidelity.autokernel/workload/types.py (2)
83-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_FIDELITY_FIELDSduplicates the field set inFidelityBudget.from_dict.
autokernel/verification/fidelity.pyLines 300-307 declares the same six field names inline. TheFidelitySpecdocstring at Lines 590-592 states that validation lives inFidelityBudgetso the contract and its gate cannot drift apart, but this field list drifts independently of it.Adding a field to one location and not the other makes one layer reject what the other accepts. Export the set from
fidelity.pyand reuse it here.♻️ Proposed direction
In
autokernel/verification/fidelity.py, name the set and use it infrom_dict:#: Fields a declared fidelity budget may carry. The workload manifest layer #: reuses this so the two cannot drift apart. BUDGET_FIELDS = frozenset( {"tier", "min_ssim", "max_lpips", "min_vbench", "frame_set", "seed"} )Then in this file:
-_FIDELITY_FIELDS = { - "tier", - "min_ssim", - "max_lpips", - "min_vbench", - "frame_set", - "seed", -} +# Imported lazily inside FidelitySpec.from_dict to match the existing +# deferred-import pattern and avoid a package import cycle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autokernel/workload/types.py` around lines 83 - 90, Export a shared `BUDGET_FIELDS` set from `FidelityBudget.from_dict`’s module in `autokernel/verification/fidelity.py`, and update `FidelityBudget.from_dict` to use it for validation. Replace the local `_FIDELITY_FIELDS` definition in the workload types module with an import and reuse `BUDGET_FIELDS`, preserving the existing field names and validation behavior.
628-631: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the real return type on
budget().The annotation is
"Any", but the docstring states the method returns aFidelityBudget. Callers and type checkers lose the type. The deferred import is the cause, andTYPE_CHECKINGresolves it without a runtime import.♻️ Proposed change
Add at module scope:
if TYPE_CHECKING: from ..verification.fidelity import FidelityBudgetThen:
- def budget(self) -> "Any": + def budget(self) -> "FidelityBudget": """Return the validated :class:`FidelityBudget` this spec describes.""" from ..verification.fidelity import FidelityBudget🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autokernel/workload/types.py` around lines 628 - 631, Update the budget method’s return annotation from "Any" to "FidelityBudget", and add a module-scope TYPE_CHECKING import for FidelityBudget so typing resolves without introducing a runtime import. Preserve the existing runtime deferred import and validation behavior in budget.autokernel/verification/__init__.py (1)
45-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument or re-export the perceptual backend APIs.
autokernel/verification/__init__.pyre-exportsFrameSet,MetricUnavailable,PerceptualError,compare_frame_sets, andssimfromperceptual.py, butperceptual.pyalso exportslpips_backendandvbench_backendwithout package-level re-exports. If callers are expected to use those backend factories, add them to the package surface; otherwise remove them fromautokernel/verification/perceptual.py’s__all__.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autokernel/verification/__init__.py` around lines 45 - 65, Align the package API for the backend factories: either re-export lpips_backend and vbench_backend from autokernel.verification alongside the existing perceptual symbols, or remove them from perceptual.py’s __all__ if they are not intended for callers. Keep the chosen public API consistent between perceptual.py and the package initializer.modal/verify_tiered_fidelity.py (1)
144-155: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
tier_gates_onlyrequests a GPU for CPU-only tests.This function runs only
tests/test_fidelity_tiers.pyandtests/test_perceptual_harness.py. Both files' own headers state they are "Numpy only -- no torch" and "never import torch." Requestinggpu="H100!"for this run adds scheduling latency and cost without benefit, and works against the function's own stated purpose: "Faster feedback loop while iterating on the contract itself."Drop the GPU requirement for this function.
♻️ Proposed fix
-@app.function(image=image, gpu="H100!", timeout=15 * 60, scaledown_window=60) +@app.function(image=image, timeout=15 * 60, scaledown_window=60) def tier_gates_only() -> str:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modal/verify_tiered_fidelity.py` around lines 144 - 155, Remove the GPU requirement from the tier_gates_only function decorator while preserving its timeout, scaling, test selection, and return behavior; this CPU-only test path should no longer request H100 resources.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@autokernel/optimize/adapters.py`:
- Around line 96-100: Update the PerceptualEvidence construction in the
measurement parsing flow to fail closed when raw lacks stage_status: require the
key explicitly or use the existing non-success/held status value, never default
to "ok". Preserve successful handling when a valid stage_status is present and
ensure missing evidence remains held without crashing the campaign.
In `@autokernel/verification/fidelity.py`:
- Around line 118-132: Update _check_threshold to accept a subject label and use
it in validation error messages instead of always saying “fidelity threshold.”
Pass the existing threshold subject from threshold callers, and call it with
subject="perceptual evidence" from PerceptualEvidence.__post_init__ so malformed
measurements identify the harness output.
- Around line 527-564: Add an optional expected frame-count field to
FidelityBudget and FidelitySpec, and include it in _FIDELITY_FIELDS so
configuration can declare the contract. In the fidelity verification flow
alongside the frame_set and seed checks, reject evidence when the declared count
is set and evidence.frames_compared differs, returning a held FidelityVerdict
with evidence_required=True. Preserve existing behavior when no frame count is
declared.
In `@autokernel/verification/perceptual.py`:
- Around line 162-168: Update the public ssim input handling to call
_as_float_array on reference and candidate directly before adding the leading
axis, rather than applying [None, ...] to the raw arguments. Preserve the
existing labels and subsequent shape validation while ensuring list and tuple
inputs raise PerceptualError through the conversion path.
- Around line 263-275: Update compare_frame_sets to validate that reference.name
and candidate.name match, alongside the existing seed and shape checks. Raise
PerceptualError with both names when they differ, before producing comparison
evidence so evaluate_fidelity cannot accept a substituted frame set.
- Around line 121-133: Update _as_float_array to enforce its documented float64
[0, 1] output for both integer and floating-point inputs. Do not silently scale
floating-point values or normalize signed integers using only iinfo.max;
instead, validate the converted range and raise PerceptualError with the
existing label when any finite value falls outside [0, 1], while preserving the
non-finite check.
- Around line 296-303: Clamp worst_ssim to the documented valid range of [-1.0,
1.0] immediately before constructing PerceptualEvidence, preserving the computed
value when it is already in range and preventing tiny floating-point overshoots
from reaching __post_init__._check_threshold().
In `@docs/TIERED_FIDELITY.md`:
- Around line 93-101: Update the evidence object example in TIERED_FIDELITY.md
to include the stage_status field emitted by PerceptualEvidence.as_dict,
preserving the existing frame_set, seed, and frames_compared fields.
---
Nitpick comments:
In `@autokernel/verification/__init__.py`:
- Around line 45-65: Align the package API for the backend factories: either
re-export lpips_backend and vbench_backend from autokernel.verification
alongside the existing perceptual symbols, or remove them from perceptual.py’s
__all__ if they are not intended for callers. Keep the chosen public API
consistent between perceptual.py and the package initializer.
In `@autokernel/verification/fidelity.py`:
- Around line 321-342: Update FidelityBudget.from_workload to detect the
workload’s FidelitySpec and return its validated budget via the spec’s budget()
accessor, before the field-by-field fallback branch. Preserve existing handling
for FidelityBudget, mappings, and workloads without fidelity.
In `@autokernel/verification/perceptual.py`:
- Around line 190-218: Update lpips_backend to cache the initialized LPIPS
network/backend for reuse across calls, ensuring pretrained AlexNet weights are
loaded only once per process while preserving MetricUnavailable behavior.
Replace the to_nchw lambda inside _score with a local def and keep the existing
tensor conversion and scoring behavior unchanged.
- Around line 282-294: Update the LPIPS scoring path in compare_frame_sets so
failures while invoking scorer or aggregating its results are caught and treated
as an absent measurement by leaving worst_lpips as None. Preserve the existing
MetricUnavailable handling and ensure only the documented PerceptualError
contract escapes from this function.
- Around line 136-152: Replace the dense 2-D Gaussian path in _gaussian_window
and _filter2d with separable filtering using a normalized 1-D kernel from
_gaussian_window_1d, applying horizontal and vertical correlations while
preserving valid-mode dimensions and size validation. Update ssim to pass the
1-D kernel through this path, and verify numerical equivalence against the
documented scikit-image cross-check.
In `@autokernel/workload/types.py`:
- Around line 83-90: Export a shared `BUDGET_FIELDS` set from
`FidelityBudget.from_dict`’s module in `autokernel/verification/fidelity.py`,
and update `FidelityBudget.from_dict` to use it for validation. Replace the
local `_FIDELITY_FIELDS` definition in the workload types module with an import
and reuse `BUDGET_FIELDS`, preserving the existing field names and validation
behavior.
- Around line 628-631: Update the budget method’s return annotation from "Any"
to "FidelityBudget", and add a module-scope TYPE_CHECKING import for
FidelityBudget so typing resolves without introducing a runtime import. Preserve
the existing runtime deferred import and validation behavior in budget.
In `@modal/verify_tiered_fidelity.py`:
- Around line 144-155: Remove the GPU requirement from the tier_gates_only
function decorator while preserving its timeout, scaling, test selection, and
return behavior; this CPU-only test path should no longer request H100
resources.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 67c3d1a0-c0cc-41bc-af79-bcc76233ab90
📒 Files selected for processing (12)
autokernel/artifact/finalizer.pyautokernel/artifact/types.pyautokernel/optimize/adapters.pyautokernel/verification/__init__.pyautokernel/verification/fidelity.pyautokernel/verification/perceptual.pyautokernel/workload/__init__.pyautokernel/workload/types.pydocs/TIERED_FIDELITY.mdmodal/verify_tiered_fidelity.pytests/test_fidelity_tiers.pytests/test_perceptual_harness.py
| ssim=raw.get("ssim"), | ||
| lpips=raw.get("lpips"), | ||
| vbench=raw.get("vbench"), | ||
| stage_status=str(raw.get("stage_status", "ok")), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
stage_status defaults to "ok" instead of failing closed.
Every other field in this constructor call fails closed when absent: frame_set defaults to "", which PerceptualEvidence.__post_init__ rejects as non-empty-required, and seed/frames_compared default to None, which fails the integer check. stage_status, however, defaults to "ok" on Line 99. If a measurement producer omits this key (a schema mismatch or a partial write), the evidence is treated as a completed, successful measurement instead of being held.
This contradicts the function's own docstring: "a broken measurement must hold the artifact, not crash the campaign." A missing stage_status is exactly the "missing evidence" case that the rest of this PR is built to fail closed on.
Default to a value that keeps the gate closed, or require the key explicitly.
🛡️ Proposed fix
lpips=raw.get("lpips"),
vbench=raw.get("vbench"),
- stage_status=str(raw.get("stage_status", "ok")),
+ stage_status=str(raw.get("stage_status", "failed")),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ssim=raw.get("ssim"), | |
| lpips=raw.get("lpips"), | |
| vbench=raw.get("vbench"), | |
| stage_status=str(raw.get("stage_status", "ok")), | |
| ) | |
| ssim=raw.get("ssim"), | |
| lpips=raw.get("lpips"), | |
| vbench=raw.get("vbench"), | |
| stage_status=str(raw.get("stage_status", "failed")), | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/optimize/adapters.py` around lines 96 - 100, Update the
PerceptualEvidence construction in the measurement parsing flow to fail closed
when raw lacks stage_status: require the key explicitly or use the existing
non-success/held status value, never default to "ok". Preserve successful
handling when a valid stage_status is present and ensure missing evidence
remains held without crashing the campaign.
| def _check_threshold(name: str, value: Any) -> float | None: | ||
| if value is None: | ||
| return None | ||
| if isinstance(value, bool) or not isinstance(value, (int, float)): | ||
| raise FidelityError(f"fidelity threshold {name} must be a number, got {value!r}") | ||
| number = float(value) | ||
| if not math.isfinite(number): | ||
| raise FidelityError(f"fidelity threshold {name} must be finite, got {value!r}") | ||
| low, high = _METRIC_RANGES[name] | ||
| if not low <= number <= high: | ||
| raise FidelityError( | ||
| f"fidelity threshold {name}={number!r} is outside the metric's " | ||
| f"range [{low}, {high}]" | ||
| ) | ||
| return number |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the validation message reflect the caller.
_check_threshold also validates measured values in PerceptualEvidence.__post_init__ (Line 386). A malformed measurement then reports "fidelity threshold ssim must be a number", which points the reader at the workload contract instead of the harness output. Pass a label so each caller names its own subject.
♻️ Proposed change
-def _check_threshold(name: str, value: Any) -> float | None:
+def _check_threshold(name: str, value: Any, *, subject: str = "fidelity threshold") -> float | None:
if value is None:
return None
if isinstance(value, bool) or not isinstance(value, (int, float)):
- raise FidelityError(f"fidelity threshold {name} must be a number, got {value!r}")
+ raise FidelityError(f"{subject} {name} must be a number, got {value!r}")
number = float(value)
if not math.isfinite(number):
- raise FidelityError(f"fidelity threshold {name} must be finite, got {value!r}")
+ raise FidelityError(f"{subject} {name} must be finite, got {value!r}")
low, high = _METRIC_RANGES[name]
if not low <= number <= high:
raise FidelityError(
- f"fidelity threshold {name}={number!r} is outside the metric's "
+ f"{subject} {name}={number!r} is outside the metric's "
f"range [{low}, {high}]"
)
return numberThen call it with subject="perceptual evidence" from PerceptualEvidence.__post_init__.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/verification/fidelity.py` around lines 118 - 132, Update
_check_threshold to accept a subject label and use it in validation error
messages instead of always saying “fidelity threshold.” Pass the existing
threshold subject from threshold callers, and call it with subject="perceptual
evidence" from PerceptualEvidence.__post_init__ so malformed measurements
identify the harness output.
| if budget.frame_set and evidence.frame_set != budget.frame_set: | ||
| return FidelityVerdict( | ||
| tier=budget.tier, | ||
| passed=False, | ||
| reason=( | ||
| "held: perceptual evidence was measured on frame set " | ||
| f"{evidence.frame_set!r}, but the budget contracts " | ||
| f"{budget.frame_set!r}" | ||
| ), | ||
| evidence_required=True, | ||
| ) | ||
| if budget.seed is not None and evidence.seed != budget.seed: | ||
| return FidelityVerdict( | ||
| tier=budget.tier, | ||
| passed=False, | ||
| reason=( | ||
| f"held: perceptual evidence was measured at seed {evidence.seed}, " | ||
| f"but the budget contracts seed {budget.seed}" | ||
| ), | ||
| evidence_required=True, | ||
| ) | ||
|
|
||
| missing = [ | ||
| metric | ||
| for metric in budget.declared_metrics | ||
| if evidence.value_for(metric) is None | ||
| ] | ||
| if missing: | ||
| return FidelityVerdict( | ||
| tier=budget.tier, | ||
| passed=False, | ||
| reason=( | ||
| "held: the budget gates on " | ||
| f"{', '.join(missing)} but the harness measured " | ||
| f"{', '.join(evidence.available_metrics) or 'nothing'}" | ||
| ), | ||
| evidence_required=True, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Tier 2 verifies frame-set identity but not frame count.
The gate checks frame_set and seed, then checks that every declared metric has a value. It never checks evidence.frames_compared. FidelityBudget declares no expected frame count, so evidence measured over one frame satisfies a budget contracted on wan-1.3b-fixed-seed-8.
This matters because evidence does not always come from compare_frame_sets. autokernel/optimize/adapters.py Lines 78-102 builds PerceptualEvidence directly from a validation-result mapping and reads frames_compared verbatim. The count-mismatch guard in compare_frame_sets (autokernel/verification/perceptual.py Lines 271-275) does not cover that path, and it only compares candidate against reference, not against the contracted frame set.
Add a declared frame count to the budget and enforce it here, in the same way frame_set and seed are enforced.
🛡️ Proposed direction
if budget.seed is not None and evidence.seed != budget.seed:
...
+ if budget.frames is not None and evidence.frames_compared != budget.frames:
+ return FidelityVerdict(
+ tier=budget.tier,
+ passed=False,
+ reason=(
+ f"held: perceptual evidence covers {evidence.frames_compared} "
+ f"frames, but the budget contracts {budget.frames}"
+ ),
+ evidence_required=True,
+ )This requires a new frames: int | None field on FidelityBudget, in _FIDELITY_FIELDS (autokernel/workload/types.py Lines 83-90), and on FidelitySpec.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/verification/fidelity.py` around lines 527 - 564, Add an optional
expected frame-count field to FidelityBudget and FidelitySpec, and include it in
_FIDELITY_FIELDS so configuration can declare the contract. In the fidelity
verification flow alongside the frame_set and seed checks, reject evidence when
the declared count is set and evidence.frames_compared differs, returning a held
FidelityVerdict with evidence_required=True. Preserve existing behavior when no
frame count is declared.
| if np.issubdtype(array.dtype, np.integer): | ||
| info = np.iinfo(array.dtype) | ||
| scale = float(info.max) | ||
| array = array.astype(np.float64) / scale | ||
| else: | ||
| array = array.astype(np.float64) | ||
|
|
||
| if not np.all(np.isfinite(array)): | ||
| # A NaN frame would make every metric NaN and every comparison | ||
| # meaningless. R4's comparator reported max_abs_error=nan while | ||
| # allclose returned True; that class of bug does not get a second run. | ||
| raise PerceptualError(f"{label}: frames contain NaN or infinite values") | ||
| return array |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
_as_float_array does not enforce the [0, 1] range it documents.
The docstring at Line 108 states the result is float64 in [0, 1]. Two input classes break that:
- Float input is never scaled or checked. Frames stored as float32 in [0, 255] pass through unchanged.
ssimthen appliesc1andc2derived from a dynamic range of 1.0 (Lines 171-172). Those stabilizers become negligible relative to the data, and the returned SSIM is inflated. An inflated SSIM promotes an artifact, which is the fail-open direction the module docstring rejects at Lines 21-24. - Signed integer input is scaled by
info.maxonly. Forint8,info.maxis 127 and the minimum is -128, so the result spans [-1.008, 1.0], not [0, 1].
Reject or explicitly handle out-of-range input rather than scaling on an assumption.
🛡️ Proposed fix
if np.issubdtype(array.dtype, np.integer):
info = np.iinfo(array.dtype)
+ if info.min < 0:
+ raise PerceptualError(
+ f"{label}: signed integer frames ({array.dtype}) are ambiguous; "
+ "convert to unsigned integer or float in [0, 1] first"
+ )
scale = float(info.max)
array = array.astype(np.float64) / scale
else:
array = array.astype(np.float64)
if not np.all(np.isfinite(array)):
# A NaN frame would make every metric NaN and every comparison
# meaningless. R4's comparator reported max_abs_error=nan while
# allclose returned True; that class of bug does not get a second run.
raise PerceptualError(f"{label}: frames contain NaN or infinite values")
+
+ low, high = float(array.min()), float(array.max())
+ if low < 0.0 or high > 1.0:
+ raise PerceptualError(
+ f"{label}: frames must be in [0, 1] after normalization, "
+ f"got [{low}, {high}]"
+ )
return array📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if np.issubdtype(array.dtype, np.integer): | |
| info = np.iinfo(array.dtype) | |
| scale = float(info.max) | |
| array = array.astype(np.float64) / scale | |
| else: | |
| array = array.astype(np.float64) | |
| if not np.all(np.isfinite(array)): | |
| # A NaN frame would make every metric NaN and every comparison | |
| # meaningless. R4's comparator reported max_abs_error=nan while | |
| # allclose returned True; that class of bug does not get a second run. | |
| raise PerceptualError(f"{label}: frames contain NaN or infinite values") | |
| return array | |
| if np.issubdtype(array.dtype, np.integer): | |
| info = np.iinfo(array.dtype) | |
| if info.min < 0: | |
| raise PerceptualError( | |
| f"{label}: signed integer frames ({array.dtype}) are ambiguous; " | |
| "convert to unsigned integer or float in [0, 1] first" | |
| ) | |
| scale = float(info.max) | |
| array = array.astype(np.float64) / scale | |
| else: | |
| array = array.astype(np.float64) | |
| if not np.all(np.isfinite(array)): | |
| # A NaN frame would make every metric NaN and every comparison | |
| # meaningless. R4's comparator reported max_abs_error=nan while | |
| # allclose returned True; that class of bug does not get a second run. | |
| raise PerceptualError(f"{label}: frames contain NaN or infinite values") | |
| low, high = float(array.min()), float(array.max()) | |
| if low < 0.0 or high > 1.0: | |
| raise PerceptualError( | |
| f"{label}: frames must be in [0, 1] after normalization, " | |
| f"got [{low}, {high}]" | |
| ) | |
| return array |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/verification/perceptual.py` around lines 121 - 133, Update
_as_float_array to enforce its documented float64 [0, 1] output for both integer
and floating-point inputs. Do not silently scale floating-point values or
normalize signed integers using only iinfo.max; instead, validate the converted
range and raise PerceptualError with the existing label when any finite value
falls outside [0, 1], while preserving the non-finite check.
| np = _numpy() | ||
| ref = _as_float_array(reference[None, ...], label="reference")[0] | ||
| cand = _as_float_array(candidate[None, ...], label="candidate")[0] | ||
| if ref.shape != cand.shape: | ||
| raise PerceptualError( | ||
| f"frame shape mismatch: reference {ref.shape} vs candidate {cand.shape}" | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
ssim requires an ndarray despite accepting Any.
Lines 163-164 apply [None, ...] to the raw arguments before _as_float_array converts them. NumPy indexing syntax is not valid on a list or tuple, so ssim([[...]], [[...]]) raises TypeError instead of PerceptualError. ssim is public: it is listed in __all__ at Line 48 and re-exported from autokernel/verification/__init__.py at Line 127.
Convert first, then add the axis.
🛡️ Proposed fix
np = _numpy()
- ref = _as_float_array(reference[None, ...], label="reference")[0]
- cand = _as_float_array(candidate[None, ...], label="candidate")[0]
+ ref = _as_float_array(np.asarray(reference)[None, ...], label="reference")[0]
+ cand = _as_float_array(np.asarray(candidate)[None, ...], label="candidate")[0]This matches the pattern already used in lpips_backend at Lines 211-212.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| np = _numpy() | |
| ref = _as_float_array(reference[None, ...], label="reference")[0] | |
| cand = _as_float_array(candidate[None, ...], label="candidate")[0] | |
| if ref.shape != cand.shape: | |
| raise PerceptualError( | |
| f"frame shape mismatch: reference {ref.shape} vs candidate {cand.shape}" | |
| ) | |
| np = _numpy() | |
| ref = _as_float_array(np.asarray(reference)[None, ...], label="reference")[0] | |
| cand = _as_float_array(np.asarray(candidate)[None, ...], label="candidate")[0] | |
| if ref.shape != cand.shape: | |
| raise PerceptualError( | |
| f"frame shape mismatch: reference {ref.shape} vs candidate {cand.shape}" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/verification/perceptual.py` around lines 162 - 168, Update the
public ssim input handling to call _as_float_array on reference and candidate
directly before adding the leading axis, rather than applying [None, ...] to the
raw arguments. Preserve the existing labels and subsequent shape validation
while ensuring list and tuple inputs raise PerceptualError through the
conversion path.
| if reference.seed != candidate.seed: | ||
| raise PerceptualError( | ||
| f"frame sets were generated at different seeds " | ||
| f"({reference.seed} vs {candidate.seed}); a perceptual comparison " | ||
| f"is only meaningful at a fixed seed" | ||
| ) | ||
| ref_frames = _as_float_array(reference.frames, label="reference") | ||
| cand_frames = _as_float_array(candidate.frames, label="candidate") | ||
| if ref_frames.shape != cand_frames.shape: | ||
| raise PerceptualError( | ||
| f"frame set shape mismatch: reference {ref_frames.shape} vs " | ||
| f"candidate {cand_frames.shape}" | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
compare_frame_sets does not check that the two frame sets have the same name.
The function rejects a seed mismatch (Lines 263-268) and a shape mismatch (Lines 271-275). It never compares reference.name against candidate.name. Line 297 then records candidate.name as the evidence frame_set.
A candidate named wan-1.3b-fixed-seed-8 compared against a reference from a different frame set therefore produces evidence that satisfies a budget contracting wan-1.3b-fixed-seed-8. evaluate_fidelity checks the recorded name against the budget (autokernel/verification/fidelity.py Lines 527-537), so it cannot detect the substitution. This is the same defect class the FrameSet docstring rejects at Lines 87-89.
🛡️ Proposed fix
if reference.seed != candidate.seed:
raise PerceptualError(
f"frame sets were generated at different seeds "
f"({reference.seed} vs {candidate.seed}); a perceptual comparison "
f"is only meaningful at a fixed seed"
)
+ if reference.name != candidate.name:
+ raise PerceptualError(
+ f"frame sets have different names ({reference.name!r} vs "
+ f"{candidate.name!r}); the evidence would be recorded under the "
+ f"candidate's name and would not describe this reference"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if reference.seed != candidate.seed: | |
| raise PerceptualError( | |
| f"frame sets were generated at different seeds " | |
| f"({reference.seed} vs {candidate.seed}); a perceptual comparison " | |
| f"is only meaningful at a fixed seed" | |
| ) | |
| ref_frames = _as_float_array(reference.frames, label="reference") | |
| cand_frames = _as_float_array(candidate.frames, label="candidate") | |
| if ref_frames.shape != cand_frames.shape: | |
| raise PerceptualError( | |
| f"frame set shape mismatch: reference {ref_frames.shape} vs " | |
| f"candidate {cand_frames.shape}" | |
| ) | |
| if reference.seed != candidate.seed: | |
| raise PerceptualError( | |
| f"frame sets were generated at different seeds " | |
| f"({reference.seed} vs {candidate.seed}); a perceptual comparison " | |
| f"is only meaningful at a fixed seed" | |
| ) | |
| if reference.name != candidate.name: | |
| raise PerceptualError( | |
| f"frame sets have different names ({reference.name!r} vs " | |
| f"{candidate.name!r}); the evidence would be recorded under the " | |
| f"candidate's name and would not describe this reference" | |
| ) | |
| ref_frames = _as_float_array(reference.frames, label="reference") | |
| cand_frames = _as_float_array(candidate.frames, label="candidate") | |
| if ref_frames.shape != cand_frames.shape: | |
| raise PerceptualError( | |
| f"frame set shape mismatch: reference {ref_frames.shape} vs " | |
| f"candidate {cand_frames.shape}" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/verification/perceptual.py` around lines 263 - 275, Update
compare_frame_sets to validate that reference.name and candidate.name match,
alongside the existing seed and shape checks. Raise PerceptualError with both
names when they differ, before producing comparison evidence so
evaluate_fidelity cannot accept a substituted frame set.
| return PerceptualEvidence( | ||
| frame_set=candidate.name, | ||
| seed=candidate.seed, | ||
| frames_compared=count, | ||
| ssim=worst_ssim, | ||
| lpips=worst_lpips, | ||
| vbench=vbench_score, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Reproduce the SSIM implementation and check for >1.0 overshoot
# on identical frames across dtypes, sizes, and content.
set -euo pipefail
pip install --quiet "numpy==2.2.0"
FILE=$(fd --type f 'perceptual.py' autokernel 2>/dev/null | head -1)
echo "Reviewing: ${FILE:-not found}"
[ -n "${FILE:-}" ] && sed -n '136,190p' "$FILE"
python - <<'PY'
import numpy as np
_WINDOW_SIZE, _WINDOW_SIGMA, _K1, _K2 = 11, 1.5, 0.01, 0.03
def window():
c = np.arange(_WINDOW_SIZE, dtype=np.float64) - (_WINDOW_SIZE - 1) / 2.0
k = np.exp(-(c**2) / (2.0 * _WINDOW_SIGMA**2))
k /= k.sum()
return np.outer(k, k)
def filt(plane, w):
v = np.lib.stride_tricks.sliding_window_view(plane, (11, 11))
return np.einsum("ijkl,kl->ij", v, w)
def ssim(ref, cand):
w = window()
c1, c2 = _K1**2, _K2**2
scores = []
for ch in range(ref.shape[-1]):
x, y = ref[..., ch], cand[..., ch]
mx, my = filt(x, w), filt(y, w)
mxs, mys, mxy = mx*mx, my*my, mx*my
sx = filt(x*x, w) - mxs
sy = filt(y*y, w) - mys
sxy = filt(x*y, w) - mxy
num = (2*mxy + c1) * (2*sxy + c2)
den = (mxs + mys + c1) * (sx + sy + c2)
scores.append(float(np.mean(num / den)))
return float(sum(scores) / len(scores))
worst = 0.0
rng = np.random.default_rng(0)
for trial in range(40):
h = int(rng.integers(11, 96)); wd = int(rng.integers(11, 96))
ch = int(rng.integers(1, 4))
frame = rng.random((h, wd, ch))
s = ssim(frame, frame.copy())
worst = max(worst, s)
if s > 1.0:
print(f"OVERSHOOT trial={trial} shape=({h},{wd},{ch}) ssim={s!r} excess={s-1.0:.3e}")
# Also test flat and near-flat frames, where variance terms are tiny.
for value in (0.0, 0.5, 1.0):
flat = np.full((64, 64, 3), value)
s = ssim(flat, flat.copy())
worst = max(worst, s)
print(f"flat={value} ssim={s!r} >1.0={s > 1.0}")
print(f"max observed SSIM on identical frames: {worst!r}")
print(f"exceeds 1.0: {worst > 1.0}")
PYRepository: aryan5v/motionkernel
Length of output: 999
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files and print snippets/tests without installing external dependencies.
file=$(git ls-files | awk '/autokernel\/verification\/perceptual\.py$/{print; exit}')
echo "FILE=$file"
if [ -n "$file" ]; then
wc -l "$file"
sed -n '120,210p' "$file"
echo "---"
sed -n '280,310p' "$file"
fi
echo "--- search usages/defaults ---"
rg -n "class PerceptualEvidence|def _check_threshold|def ssim|frames_compared|ssim" autokernel/verification -S || trueRepository: aryan5v/motionkernel
Length of output: 7550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether NumPy without pip is available for a quick numerical probe.
python3 - <<'PY'
try:
import numpy as np
print(f"numpy={np.__version__}")
except Exception as exc:
print(f"numpy_unavailable={type(exc).__name__}: {exc}")
PY
if python3 - <<'PY'
import numpy as np
PY
then
python3 - <<'PY'
import numpy as np
_WINDOW_SIZE, _WINDOW_SIGMA, _K1, _K2 = 11, 1.5, 0.01, 0.03
def window():
c = np.arange(_WINDOW_SIZE, dtype=np.float64) - (_WINDOW_SIZE - 1) / 2.0
k = np.exp(-(c**2) / (2.0 * _WINDOW_SIGMA**2))
k /= k.sum()
return np.outer(k, k)
def filt(plane, w):
v = np.lib.stride_tricks.sliding_window_view(plane, (11, 11))
return np.einsum("ijkl,ki->ij", v, w)
def ssim(ref, cand):
w = window()
c1, c2 = _K1**2, _K2**2
scores = []
for ch in range(ref.shape[-1]):
x, y = ref[..., ch], cand[..., ch]
mx, my = filt(x, w), filt(y, w)
mxs, mys, mxy = mx*mx, my*my, mx*my
sx = filt(x*x, w) - mxs
sy = filt(y*y, w) - mys
sxy = filt(x*y, w) - mxy
num = (2*mxy + c1) * (2*sxy + c2)
den = (mxs + mys + c1) * (sx + sy + c2)
scores.append(float(np.mean(num / den)))
return float(sum(scores) / len(scores))
worst = 0.0
rng = np.random.default_rng(0)
overshoots = []
for trial in range(1000):
h = int(rng.integers(11, 96)); wd = int(rng.integers(11, 96))
ch = int(rng.integers(1, 4))
frame = rng.random((h, wd, ch))
s = ssim(frame, frame.copy())
worst = max(worst, s)
if s > 1.0:
overshoots.append((trial, h, wd, ch, s, s - 1.0))
for value in (0.0, 0.5, 1.0, 1e-16):
flat = np.full((64, 64, 3), value)
s = ssim(flat, flat.copy())
worst = max(worst, s)
print(f"flat={value} ssim={s!r} >1.0={s > 1.0} excess={s-1.0:.3e}")
print(f"max observed SSIM on identical frames: {worst!r}")
print(f"exceeds 1.0: {worst > 1.0}")
print(f"overshoot_count: {len(overshoots)}")
for row in overshoots[:10]:
print(row)
PY
fiRepository: aryan5v/motionkernel
Length of output: 339
🌐 Web query:
scikit image structural_similarity float error near 1.0 identical images documentation
💡 Result:
The common issue where skimage.metrics.structural_similarity produces unexpected results (including potential errors or incorrect indices near 1.0) when using floating-point images is primarily caused by the incorrect automatic estimation of the data_range parameter [1][2][3]. For floating-point image data, the function attempts to guess the data_range based on the image's data type. Because scikit-image's internal dtype_range defines a range of [-1.0, 1.0] for float types, the function often defaults to a data_range of 2.0 instead of the 1.0 commonly expected for normalized image data [1][2]. This incorrect range estimation can lead to artificially high or otherwise inaccurate SSIM values, even for identical images [2]. To resolve this, you should always explicitly specify the data_range argument when working with floating-point images [1][3]. Recommended practices: 1. Specify data_range: Pass the scalar difference between the maximum and minimum possible values of your image data (e.g., data_range=1.0 if your data is in the [1] range, or data_range=255 if it is in the range) [1][4][5]. 2. Use peak-to-peak if unsure: If you do not know the theoretical data range, you can use the peak-to-peak value of your input image as a proxy, such as data_range=image.ptp() [6]. 3. Avoid invalid arguments: Ensure you pass a scalar value to data_range. Passing a tuple or other sequence, such as (1.0, 1.0), will cause a TypeError (e.g., "can't multiply sequence by non-int of type float") [6]. For more accurate results that better match standard implementations like those by Wang et al., it is also recommended to set gaussian_weights=True, sigma=1.5, and use_sample_covariance=False along with an explicit data_range [1][4][3].
Citations:
- 1: https://scikit-image.org/docs/stable/api/skimage.metrics
- 2: incorrect ssim result? scikit-image/scikit-image#3942
- 3: https://scikit-image.org/docs/stable/api/skimage.metrics.html?highlight=compare_mse
- 4: https://stackoverflow.com/questions/58604326/which-ssim-is-correct-skimage-metrics-structural-similarity
- 5: skimage.metrics structural_similarity() VS Original MatLab version scikit-image/scikit-image#4278
- 6: TypeError "can't multiply sequence by non-int of type float" in
structural_similarityscikit-image/scikit-image#7342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'TIERED_FIDELITY.md|perceptual_harness.py' . | sed 's#^\./##'
echo "---"
sed -n '1,150p' docs/TIERED_FIDELITY.md
echo "---"
if [ -x "$(command -v ast-grep)" ]; then
ast-grep outline autokernel/verification/fidelity.py --match PerceptualEvidence --view expanded || true
ast-grep run --pattern '$CLASS.__post_init__($$CALL)' --lang python autokernel/verification/fidelity.py || true
fi
echo "---"
sed -n '118,135p' autokernel/verification/fidelity.py
sed -n '337,405p' autokernel/verification/fidelity.pyRepository: aryan5v/motionkernel
Length of output: 9764
Ensure SSIM evidence cannot construct out of range
PerceptualEvidence.__post_init__ runs _check_threshold() on ssim, and ssim has range [-1.0, 1.0]. This path can be invalid when the SSIM numerator/denominator accumulate as an exact 1.0 plus a tiny floating-point overshoot. If the documented ~1e-16 agreement with scikit-image means this can happen, clamp worst_ssim before returning PerceptualEvidence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/verification/perceptual.py` around lines 296 - 303, Clamp
worst_ssim to the documented valid range of [-1.0, 1.0] immediately before
constructing PerceptualEvidence, preserving the computed value when it is
already in range and preventing tiny floating-point overshoots from reaching
__post_init__._check_threshold().
| ```json | ||
| "fidelity": { | ||
| "budget": {"tier": "perceptual", "tier_number": 2, "min_ssim": 0.98, ...}, | ||
| "verdict": {"passed": true, "margins": [ | ||
| {"metric": "ssim", "value": 0.9912, "threshold": 0.98, "margin": 0.0112, "passed": true}, | ||
| {"metric": "lpips", "value": 0.0071, "threshold": 0.02, "margin": 0.0129, "passed": true}]}, | ||
| "evidence": {"frame_set": "wan-1.3b-fixed-seed-8", "seed": 1234, "frames_compared": 8} | ||
| } | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The evidence example omits stage_status.
PerceptualEvidence.as_dict always emits stage_status (autokernel/verification/fidelity.py Lines 406-411). Line 99 shows frame_set, seed, and frames_compared with no ellipsis, so a reader treats it as the complete object. Line 95 does use ... for the truncated budget, which makes the omission on Line 99 read as deliberate completeness.
📝 Proposed fix
- "evidence": {"frame_set": "wan-1.3b-fixed-seed-8", "seed": 1234, "frames_compared": 8}
+ "evidence": {"frame_set": "wan-1.3b-fixed-seed-8", "seed": 1234,
+ "frames_compared": 8, "stage_status": "ok"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```json | |
| "fidelity": { | |
| "budget": {"tier": "perceptual", "tier_number": 2, "min_ssim": 0.98, ...}, | |
| "verdict": {"passed": true, "margins": [ | |
| {"metric": "ssim", "value": 0.9912, "threshold": 0.98, "margin": 0.0112, "passed": true}, | |
| {"metric": "lpips", "value": 0.0071, "threshold": 0.02, "margin": 0.0129, "passed": true}]}, | |
| "evidence": {"frame_set": "wan-1.3b-fixed-seed-8", "seed": 1234, "frames_compared": 8} | |
| } | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/TIERED_FIDELITY.md` around lines 93 - 101, Update the evidence object
example in TIERED_FIDELITY.md to include the stage_status field emitted by
PerceptualEvidence.as_dict, preserving the existing frame_set, seed, and
frames_compared fields.
1 participant
Track B of the roadmap. Unblocks Tracks A (attention backends) and C (caching), neither of which can promote under a
byte_equal-only world.The problem
parity.policycontracts the numeric axis. For a fused kernel meant as a drop-in replacement,byte_equalis correct. For an alternative attention backend (different accumulation order) or a cross-step cache (skips recomputing a step outright), it is a wall: both are rejected before they are ever benchmarked.Widening the tolerance until they fit is the R4 failure exactly — four VAE artifacts on
rcp.approx.ftz.f32reached packaging carrying up to 131072.0 of absolute error atmatch=true. A tolerance wide enough to admit a cache hit is wider still.So this doesn't widen the numeric axis. It adds a second one.
The tiers
exactperceptualadvisoryA workload declaring nothing gets tier 1. Tier 3 never auto-promotes.
The one subtle bit
At tier 2 the fidelity verdict replaces the parity check in
GenerationOutcome.decide()rather than stacking on it. A tier-2 candidate failsbyte_equalby construction; gating on parity first would quarantine every attention and caching candidate before its evidence was read.Tier 1 behaviour is unchanged down to the wording of the quarantine reason —
test_tier_1_behaviour_is_unchanged_by_the_tier_machinerypins it.Refusing to guess
Thresholds are per-workload and never invented by the harness (the rule
resolve_leaf_tolerancealready enforces; a 1.3B at 480p and a 14B at 720p don't share a perceptual noise floor). Beyond that:perceptualbudget with no threshold or no frame set is rejected at construction — a tier with no bar is no contract;Promotions record the signed margin per metric (positive always = passing, whichever way the metric runs), so a perceptual promotion is auditable from the manifest alone.
Harness
Aggregation is worst-frame, not mean — a cache perfect on 7 frames and destroying the 8th averages to a comfortable pass, and one broken frame is exactly what a schedule transform produces.
Exit criteria
Both covered as tests:
test_intentionally_lossy_control_is_rejected_at_tier_2— the control is fast (1.21×), cleanly dispatched, classifiedimproved. Everything the speed gate looks at says promote. It is quarantined on SSIM/LPIPS alone.test_known_good_approximate_artifact_passes_with_recorded_margins— a bit-inexact but perceptually clean artifact promotes, margins in the manifest.Testing
Verified on real GPUs (the earlier laptop run could not execute anything that
imports torch). Same suite, three configurations:
2b1da46(main, baseline)30e9bc9(this branch)30e9bc9(this branch)805 − 770 = 35, exactly the tests added here. The two failures
(
test_fx_region_capture[symbolic],test_gpu_smoke[matmul]) reproduceidentically at the merge-base and on both architectures, so they are
pre-existing on
mainand unrelated to this change — established by runningthe baseline rather than by arguing the change could not have caused them.
sm100 is included because it is the arch the promoted artifacts actually
target; Modal only offers sm90, so an H100-only result would not have covered
it.
modal/verify_tiered_fidelity.pycarries the harness, including thebaseline_suiteentrypoint used for the comparison above.Still not exercised: the
adapters.pywiring that carries a budget from aworkload manifest to the gate has not been run against a real campaign — that
needs a tier-2 workload and a full generation, which is Track A/C work.
Note on Track D
While checking premises I found two that no longer hold, both documented in
docs/LTX_V1_R4_ROOT_CAUSE.md§7–9:torch.cuda.CUDAGraphcapture (chosen over a compiler backend because a graph replay is bitwise-identical by construction). Gate 5 passes at 1.2514× median, 15 runs/arm, SLURM 999.byte_equaldeterministically. Both disqualifiers are independent of dispatch cost.Summary by CodeRabbit
New Features
Documentation