Skip to content

Fix SequenceEmpty dtype parameter ignored in eager mode#2949

Open
PratikWayase wants to merge 2 commits into
microsoft:mainfrom
PratikWayase:fix/sequence-empty-dtype
Open

Fix SequenceEmpty dtype parameter ignored in eager mode#2949
PratikWayase wants to merge 2 commits into
microsoft:mainfrom
PratikWayase:fix/sequence-empty-dtype

Conversation

@PratikWayase

Copy link
Copy Markdown
Contributor

Summary

Fix SequenceEmpty dtype parameter being ignored in eager/trace mode by introducing a typed runtime representation for sequences. Fixes #1562.

Changes

As noted by @gramalingam in the issue, the root cause is that we reuse Python lists for runtime sequence values, and when the list is empty, we cannot infer a suitable ONNX type. This PR implements the first proposed fix: using a runtime representation that combines a type with the value.

  • Introduce _TypedSequence, a list subclass in onnxscript/_internal/typed_sequence.py that preserves ONNX dtype information for empty sequences
  • Modify BaseEvaluator.eval_op in onnxscript/_internal/evaluator.py to wrap SequenceEmpty results with _TypedSequence using the provided dtype attribute (defaults to FLOAT if None)
  • Modify _onnxscript_to_numpy_value in onnxscript/_internal/evaluator.py to preserve the _TypedSequence wrapper (instead of downcasting to a plain list) when preparing inputs for the ONNX runtime
  • Modify value_to_type_proto in onnxscript/_internal/utils.py to extract the dtype from _TypedSequence when inferring the ONNX TypeProto

Tests

Added OpInfo-style regression test test_sequence_empty_preserves_dtype covering:

  • FLOAT16 dtype
  • DOUBLE dtype
  • INT64 dtype

Additionally verified edge cases:

  • Missing dtype attribute (correctly defaults to FLOAT)
  • Multiple SequenceInsert operations on the same typed sequence
  • SequenceAt on a typed sequence

Files:

  • onnxscript/_internal/typed_sequence.py (new)
  • onnxscript/_internal/evaluator.py
  • onnxscript/_internal/utils.py
  • tests/eager_mode_test.py

Validation

python -m pytest tests/eager_mode_test.py -k "test_sequence_empty_preserves_dtype" -v

Passed:

tests/eager_mode_test.py::EagerModeTest_0_reference_runtime::test_sequence_empty_preserves_dtype PASSED
tests/eager_mode_test.py::EagerModeTest_1_onnxruntime::test_sequence_empty_preserves_dtype PASSED
2 passed

from typing import Any, Iterable


class _TypedSequence(list):

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 fixes eager/trace-mode handling of SequenceEmpty(dtype=...) by introducing a typed runtime sequence representation so empty sequences retain an ONNX element dtype and can safely flow into later sequence ops (e.g., SequenceInsert).

Changes:

  • Added _TypedSequence (a list subclass carrying an onnx_dtype) as an internal runtime representation for sequences with explicit element dtype.
  • Updated eager-mode evaluation to wrap SequenceEmpty outputs in _TypedSequence, and to preserve _TypedSequence when converting values for runtime execution and type inference.
  • Added an eager-mode regression test covering FLOAT16, DOUBLE, and INT64 dtype for SequenceEmpty + SequenceInsert.

Reviewed changes

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

File Description
tests/eager_mode_test.py Adds regression coverage ensuring SequenceEmpty(dtype=...) prevents SequenceInsert elem-type mismatches in eager mode.
onnxscript/_internal/utils.py Updates type inference to extract ONNX element dtype from _TypedSequence instead of defaulting empty lists to float.
onnxscript/_internal/typed_sequence.py Introduces _TypedSequence wrapper that carries onnx_dtype for empty (and subsequent) sequence values.
onnxscript/_internal/evaluator.py Wraps SequenceEmpty outputs in _TypedSequence and preserves _TypedSequence during conversion for runtime execution.

Comment on lines +268 to +270
if op.name == "SequenceEmpty":
dtype = attributes.get("dtype") or onnx.TensorProto.FLOAT
return _TypedSequence(dtype, result)
Comment on lines +266 to +270
# Handle SequenceEmpty: wrap result with _TypedSequence to preserve dtype
# for empty sequences where type cannot be inferred from elements.
if op.name == "SequenceEmpty":
dtype = attributes.get("dtype") or onnx.TensorProto.FLOAT
return _TypedSequence(dtype, result)
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.63%. Comparing base (d3489d4) to head (c8258c8).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
onnxscript/_internal/typed_sequence.py 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2949      +/-   ##
==========================================
+ Coverage   72.61%   72.63%   +0.01%     
==========================================
  Files         263      264       +1     
  Lines       32034    32057      +23     
  Branches     3013     3016       +3     
==========================================
+ Hits        23263    23283      +20     
- Misses       7748     7750       +2     
- Partials     1023     1024       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

SequenceEmpty parameter dtype seems not work

3 participants