Skip to content

Commit b4764a4

Browse files
shaoxiongduanclaude
andcommitted
fix(eval): address #1380 review (cache filename, registry, deps, warn)
- README: correct the documented cache filename to ``${FASTVIDEO_EVAL_CACHE}/fvd/real_features_{extractor}.pt`` — the file is per-extractor partitioned since CLIP and VideoMAE landed; the old ``real_features.pt`` form is never used. - test_registry: include ``common.fvd`` in ``_CORE_METRICS`` so a decorator/import-time breakage on this top-level metric surfaces in CI rather than at runtime, alongside the other structurally-similar top-level common metrics already in the tuple. - metric.dependencies: declare ``transformers`` (used by the CLIP and VideoMAE extractors). Already in base deps so the registry pre-import check passes today; futureproofs if transformers ever moves to an extra. - accumulate: ``warnings.warn`` when ``sample["reference"]`` is silently ignored because reference features are already loaded (from cache or an earlier accumulate call). Surfaces the foot-gun rather than producing a plausible-but-wrong score. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent b1f2828 commit b4764a4

3 files changed

Lines changed: 15 additions & 2 deletions

File tree

fastvideo/eval/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ ev.evaluate(samples=[
273273
# corpus result is in the returned EvalResults.corpus["common.fvd"]
274274
```
275275

276-
Reference features are cached to ``${FASTVIDEO_EVAL_CACHE}/fvd/real_features.pt``
276+
Reference features are cached to ``${FASTVIDEO_EVAL_CACHE}/fvd/real_features_{extractor}.pt``
277277
the first time ``sample["reference"]`` is passed; subsequent runs load
278278
the cache automatically. Override with ``$FASTVIDEO_FVD_REF_FEATURES``
279279
or the ``cache_path=`` constructor kwarg.

fastvideo/eval/metrics/common/fvd/metric.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ class FVDMetric(BaseMetric):
157157
requires_reference = False # uses cached real features, not per-sample ref
158158
higher_is_better = False # lower FVD = better
159159
needs_gpu = True
160-
dependencies = ["huggingface_hub", "scipy"]
160+
dependencies = ["huggingface_hub", "scipy", "transformers"]
161161

162162
def __init__(
163163
self,
@@ -234,6 +234,18 @@ def accumulate(self, sample: dict) -> None:
234234
ref = ref.unsqueeze(0)
235235
self._real_features = _extract_chunked(self._extractor, ref, self._chunk)
236236
self._save_cache(self._real_features)
237+
elif sample.get("reference") is not None:
238+
# Reference features already exist (from cache or an earlier
239+
# accumulate call). Silently dropping ``sample["reference"]``
240+
# would be a foot-gun for callers expecting per-sample-reference
241+
# semantics — warn so the mismatch surfaces.
242+
warnings.warn(
243+
"common.fvd: sample['reference'] ignored because reference features "
244+
"are already loaded (from cache or a prior accumulate call). Pass the "
245+
"entire reference set on a single accumulate() call, or pre-build the "
246+
f"cache at {self.cache_path}.",
247+
stacklevel=2,
248+
)
237249

238250
def finalize(self) -> MetricResult:
239251
"""Compute FVD from all accumulated generated features vs. real features."""

fastvideo/tests/eval/test_registry.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"common.psnr",
1919
"common.ssim",
2020
"common.lpips",
21+
"common.fvd",
2122
"optical_flow.gt_optical_flow",
2223
"optical_flow.synthetic_optical_flow",
2324
"physics_iq",

0 commit comments

Comments
 (0)