Skip to content

Fix Split num_outputs with an empty optional input - #32972

Open
Tianlei Wu (tianleiwu) wants to merge 12 commits into
mainfrom
fix-split-empty-optional-input
Open

Tianlei Wu (tianleiwu) wants to merge 12 commits into
mainfrom
fix-split-empty-optional-input

Conversation

@tianleiwu

@tianleiwu Tianlei Wu (tianleiwu) commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Allow an empty optional split input edge with the opset-18 num_outputs attribute, checking whether the second input actually exists rather than rejecting any two-slot input list. Also correct the shared native split-size calculation used by CPU fallback so valid empty final outputs work and large dimensions do not lose integer precision.

Key changes

  • Preserve optional-input handling across the constructor and provider paths.
  • Use integer ceiling division and compute the final chunk from the remaining dimension, including [1, 0] and [2, 2, 2, 2, 2, 0].
  • Validate positive output counts, attribute/output-count agreement and non-negative final chunks.
  • Add native coverage for zero final outputs, one-element inputs, zero split dimensions and dimension 16777217, using both omitted-optional-input encodings.
  • Retain the existing browser regression fixtures and their exact output assertions.

Tests

  • Freshly rebuilt common production code, Python binding and actual operator-test binary: all 37 SplitOperatorTest cases pass with no skips, including all four new regressions and existing invalid-input/count/axis cases.
  • All 17 additional native graph/transpose/quantization Split tests pass.
  • The hash-verified fresh CPU binding matches ONNX reference shapes and values in 200 executions, including 58 zero-final-output cases, both absent-input forms and dimensions above float integer precision.
  • Scoped repository linters pass.
  • Earlier constructor-change validation included a 13-test CUDA/CPU focused run on A100 and compilation for CPU, built-in CUDA and the CUDA plugin. The new common arithmetic repair is locally validated on CPU; exact-new-head browser/CUDA integration CI remains the final verification gate.

Follow-up to #32970.

@tianleiwu
Tianlei Wu (tianleiwu) marked this pull request as ready for review September 30, 2026 08:06
Copilot AI balanced review requested due to automatic review settings September 30, 2026 08:06

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.

Copilot review overview

🟡 Changes recommended

The JS Split constructor still treats the empty edge as present and rejects the valid model.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes Split-18 handling of an omitted optional split edge when num_outputs is specified.

Changes:

  • Detects actual input-edge presence across built-in and plugin kernels.
  • Adds regression coverage for an explicit empty optional edge.
File Description
onnxruntime/​core/​providers/​cpu/​tensor/​split.h Distinguishes empty optional edges from provided inputs.
onnxruntime/​test/​providers/​cpu/​tensor/​split_op_test.cc Tests even splitting with an empty optional edge.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@tianleiwu
Tianlei Wu (tianleiwu) force-pushed the fix-split-empty-optional-input branch from 3add050 to 51a89b9 Compare October 2, 2026 04:37
@apsonawane

Copy link
Copy Markdown
Contributor

Tianlei Wu (@tianleiwu) 1 webgpu test is failing can you fix that. Otherwise, the PR looks good

Tianlei Wu (tianleiwu) and others added 2 commits October 9, 2026 20:49
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allocate empty outputs without binding them in the JSEP shader. Preserve original output indices for non-empty split chunks.

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

Copy link
Copy Markdown
Contributor Author

Fixed the required Web CI failures in 379afbd. Split now allocates zero-sized outputs outside the JSEP shader and binds only non-empty chunks, preserving their original output indices (the native WebGPU implementation already uses this approach). Validation: 11 JSEP Split runtime-shape tests passed, Web TypeScript checks passed with current common declarations, and targeted ESLint passed. Merged origin/main; new-head CI is running.

Preserve CPU, plugin, CoreML and JSEP empty-input handling and all runtime shape cases. Eleven exact Split JavaScript unit tests and both required TypeScript project checks passed after restoring declared dependencies.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the repository's locked Prettier formatter to the exact file rejected by the fresh JS precheck. Formatting and scoped ESLint pass; no behavior changes to the previously validated eleven Split cases.

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

Tianlei Wu (tianleiwu) commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Maintenance update at 3143158367232f22f681ca39dae872833b5d5102: the main refresh, optional-input handling and Web regressions are retained. The shared CPU/CUDA partition repair now has actual production and plugin proof.

Integer ceiling division and checked final-tail calculation replace floating-point sizing, preserve valid zero-tail outputs, reject negative final sizes and enforce num_outputs/output-count agreement. The protected inline helper is available to CPU and CUDA/plugin preparation without changing provider ABI/layout or CUDA launch, pinned-buffer or CUDA Graph behavior.

The fresh exact314 production CPU/CUDA build passed all 41 SplitOperatorTest cases, including all four original CI-red Split18 cases, the 33-output CUDA case and dimension-product regressions, with no failures/skips. The staged Python binding passed 12 cases without failures/skips, covering both omitted-input encodings and exact count diagnostics; ten profiles prove CUDA-assigned Split with CPU fallback and graph optimization disabled. Exact empty shapes include [0,8388609], [0,8388608] and the valid outer-axis final [0,2] output.

Actual current-head Linux CUDA plugin 42607563127 completed SUCCESS. Its separate real test job129155373975 executed all41 Split cases with 41 OK and zero failed/skipped, plus the broader native provider suite. The plugin Python step ran94 tests with2 environmental skips and overall OK; this is not reported as zero-skip targeted Python proof. Generic ONNX tests in the build wrapper were intentionally skipped; the separate plugin native/Python executions are the evidence, not the noisy update-stage or missing-wheel annotation.

Actual current-head Windows CUDA plugin 42607563171 also completed SUCCESS: real build129127297227 and test129150805592 succeeded, including Run Tests and Run CUDA Plugin EP Python Tests. The CodeQL Java failure was a Gradle8.7 connection timeout before compilation; only failed jobs were retried. CodeQL42607563106 attempt2 and fresh Java job129139564483 Autobuild/Perform CodeQL Analysis both completed SUCCESS.

Actual current-head Linux CUDA42607561560 completed SUCCESS. Its separate real test job129150181469 executed exactly41 SplitOperatorTest cases, with all41 OK and no failures/skips. All three normal-CUDA/Linux-plugin/Windows-plugin hard gates now have actual executed-test proof.

The subsequent Web Debug failure first occurred at ScatterND add int32: GPUBuffer.mapAsync reported a lost device, followed by WASM-abort and stale-session-creation cascades. The relevant native-WebGPU/ScatterND surfaces are unchanged from main; no Split source defect was identified from this log. After guarding the exact current314 and completed provider, a failed-only recovery was requested. Actual attempt2 Debug129178170394 completed SUCCESS on unchanged code: the original failing case passes in all six executed modes, and the formerly failing native-WebGPU GPU-location suite completes1974 tests successfully. Successful sibling start/end timestamps remain from the original attempt despite new provider record IDs; they were preserved.

The fresh complete all-20 audit at 2026-11-09T13:46:34Z verifies this exact current314 head open, non-draft, mergeable, with zero unresolved threads and all current-head checks green. This PR's CI preparation is complete; normal reviewer approval remains external. No test, tolerance, provider fallback or assertion was weakened.

Use integer ceiling division and compute the final chunk from the remaining dimension, including empty outputs. Preserve rejection of invalid output counts and negative final chunks. Cover both omitted optional-input forms and dimensions above float integer precision in native tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CUDA local split preparation still rejected valid empty outputs and used stale final-chunk math after the common CPU correction. Share the integer partition/count validation through an inline SplitBase helper usable by monolithic and plugin CUDA builds. Extend the zero-final-output regression to the outer-axis dispatch path without excluding CUDA or weakening assertions. CUDA launch and graph-capture paths are unchanged.

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

This branch has not been deployed

No deployments
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