Skip to content

Validate FeatureVectorizer batch sizes - #31671

Merged
Akshay Sonawane (apsonawane) merged 5 commits into
mainfrom
fix/feature-vectorizer-batch-check
Aug 11, 2026
Merged

Validate FeatureVectorizer batch sizes#31671
Akshay Sonawane (apsonawane) merged 5 commits into
mainfrom
fix/feature-vectorizer-batch-check

Conversation

@apsonawane

@apsonawane Akshay Sonawane (apsonawane) commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

This pull request improves the robustness of the FeatureVectorizer operator by validating runtime input shapes before allocating or writing the output.

FeatureVectorizer input validation:

  • Enforces that all input tensors have the same batch size.
  • Rejects scalar (0-D) inputs with a clean Status error instead of indexing an empty shape.
  • Returns validation failures as Status values so exception-disabled/minimal builds do not abort the host process.

Behavior change:

Rank-1 inputs are treated as a single-row batch, as required by the ai.onnx.ml specification. Mixing a rank-1 input with a rank-2 input whose batch size is greater than one now fails instead of silently leaving additional output rows zero-filled.

Unit test improvements:

  • Verifies mismatched batch sizes are rejected.
  • Verifies scalar input is rejected for both the first input and a later variadic input.

Reject mismatched batch sizes across variadic inputs before allocating the
output buffer so later inputs cannot write past the end of the output span.
Add a regression test for the mismatched-batch case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens the CPU ML FeatureVectorizer kernel by validating that all variadic inputs share a consistent batch size (first dimension), and adds a unit test that exercises the new failure path.

Changes:

  • Added runtime validation in FeatureVectorizer::Compute to reject mismatched batch sizes across inputs.
  • Added a unit test RejectMismatchedBatchSizes to ensure mismatched batches fail with an appropriate error.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
onnxruntime/core/providers/cpu/ml/feature_vectorizer.cc Adds per-input batch-size validation in the kernel implementation.
onnxruntime/test/providers/cpu/ml/feature_vectorizer_test.cc Adds a regression test covering mismatched batch sizes.

Comment thread onnxruntime/core/providers/cpu/ml/feature_vectorizer.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/feature_vectorizer_test.cc Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@tianleiwu Tianlei Wu (tianleiwu) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

The validation is placed correctly — it runs before context->Output(0, {N, total_dimensions_}), so a bad input never allocates an output or writes partial rows. The rank check is also a genuine bug fix: a 0-D input previously reached x_dims[0] / input_dims[0] on an empty dims span, which is an out-of-bounds read rather than a clean error. And input_dims.size() == 1 ? 1 : input_dims[0] matches both the ai.onnx.ml spec ("1-D tensors are treated as [1,C]") and the row-count logic already in VectorizeTensor, so validation and execution agree on what "batch" means.

One robustness point and a couple of nits inline. Verdict: COMMENT.

Cross-cutting notes

Behavior change worth documenting. Previously a rank-1 input mixed with a rank-2 input of batch > 1 was silently accepted: VectorizeTensor copied a single row and the remaining output rows stayed zero-filled. Those models now hard-fail. This is spec-conformant and the old output was almost certainly wrong, but it is still a behavior change for previously loadable models — worth calling out in the PR description.

Description drift. The description mentions one new test; the diff adds two (RejectMismatchedBatchSizes, RejectScalarInput) plus a minimum-rank check that is not described.

Existing coverage is unaffected. BasicFunctionality, HandleInputDimensionMismatch, Batch, and BatchWith3DInput all use a consistent batch across inputs, so they remain valid under the stricter rule.

Comment thread onnxruntime/core/providers/cpu/ml/feature_vectorizer.cc Outdated
Comment thread onnxruntime/core/providers/cpu/ml/feature_vectorizer.cc Outdated
Comment thread onnxruntime/core/providers/cpu/ml/feature_vectorizer.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/feature_vectorizer_test.cc
Return validation failures as Status values so exception-disabled builds do not abort. Improve missing-input errors and cover scalar input 0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@tianleiwu Tianlei Wu (tianleiwu) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the current head after the follow-up fixes. The prior concerns are addressed: rank and batch validation now return Status instead of aborting exception-disabled builds, missing inputs have useful diagnostics, and scalar input 0 has dedicated coverage. Validation still completes before output allocation, and the batch interpretation matches VectorizeTensor. Relevant CPU, minimal-build, and lint checks are green; the two failing CUDA plugin checks are both the unrelated GatherBlockQuantized plugin test.

@apsonawane
Akshay Sonawane (apsonawane) merged commit d3abafc into main Aug 11, 2026
85 of 87 checks passed
@apsonawane
Akshay Sonawane (apsonawane) deleted the fix/feature-vectorizer-batch-check branch August 11, 2026 18:57
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.

3 participants