Skip to content

test(e2e): align feature-extraction e2e tests with #807 modality-aware logic#820

Merged
timenick merged 1 commit into
mainfrom
zhiwang/fix-fe-e2e-modality
Jun 8, 2026
Merged

test(e2e): align feature-extraction e2e tests with #807 modality-aware logic#820
timenick merged 1 commit into
mainfrom
zhiwang/fix-fe-e2e-modality

Conversation

@timenick

@timenick timenick commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

What

#807 (merged) made detect_task modality-aware and removed #786's bimodal feature-extraction io_config reverse-reconstruction (in both datasets and eval). But #786's e2e tests — still on main — assert the old bimodal behavior, so they now fail on main:

  • tests/e2e/test_quantize_e2e.py::TestPerTaskDatasets::test_feature_extraction_with_pixel_values_uses_image_dataset
  • tests/e2e/test_eval_e2e.py::TestEvalPerTask::test_image_feature_extraction[feature-extraction]

Both crash with KeyError: Dinov2Config — a vision model + --task feature-extraction now resolves to the text TextDataset, which tries to load a tokenizer DINOv2 does not have.

Why update the tests (not revert the code)

Under the merged modality-aware task vocabulary:

  • feature-extraction is the text feature task; a vision embedding model's canonical task is image-feature-extraction.
  • winml inspect / auto-detect always report image-feature-extraction for a vision model (e.g. facebook/dinov2-small) — the tool never hands a user --task feature-extraction for a vision model.
  • So explicit --task feature-extraction on a vision model is a genuine modality mismatch and is expected to error. The #786 bimodal io_config dispatch (which silently recovered modality from the ONNX inputs) was deliberately removed in #807.

The e2e tests follow the new logic:

  • quantize: test_feature_extraction_with_pixel_values_uses_image_datasettest_image_feature_extraction_uses_image_dataset, asserting --task image-feature-extractionImageDataset (the canonical vision-feature calibration path).
  • eval: drop the feature-extraction param from test_image_feature_extraction (vision feature = image-feature-extraction). Also loosen the kNN floors to sanity levels (10/25), consistent with this file's N=10 convention ("Loose floors guard against degenerate output, not magnitude") — the previous 30/60 floors flaked at top1=20 on a 10-sample kNN.

Heads-up for @zhenchaoni

This realigns the two e2e tests you added in #786 and, by extension, accepts dropping the bimodal --task feature-extraction-on-a-vision-model capability #786 introduced. That removal landed in #807 (merged); this PR only updates the tests to match. If you'd rather keep that capability working (graceful) instead of erroring on the mismatch, that means restoring the io_config dispatch — happy to do that instead. Flagging for your call.

…ty-aware logic

#807 made detect_task modality-aware and removed #786's bimodal io_config
reverse-reconstruction, but #786's e2e tests still asserted the old bimodal
behavior and now fail on main. Under the new task vocabulary feature-extraction
is text-only and a vision model's canonical task is image-feature-extraction
(what inspect/auto-detect report); explicit --task feature-extraction on a
vision model is now a mismatch that errors.

- quantize: test_feature_extraction_with_pixel_values_uses_image_dataset
  -> test_image_feature_extraction_uses_image_dataset (--task image-feature-extraction)
- eval: drop the feature-extraction param from test_image_feature_extraction;
  loosen knn floors to sanity levels (10/25) per the file's N=10 convention
@timenick timenick requested a review from a team as a code owner June 8, 2026 02:13
@timenick timenick requested a review from zhenchaoni June 8, 2026 02:14
@timenick timenick merged commit f85b988 into main Jun 8, 2026
9 checks passed
@timenick timenick deleted the zhiwang/fix-fe-e2e-modality branch June 8, 2026 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants