Repository navigation
[6058870] Fix ONNX AutoCast keep_io_types for empty I/O tensors - #1626
Conversation
When a network input/output is an empty tensor (a dimension of size 0), PrecisionConverter._add_cast "fake-cast" it by retyping its value_info in place instead of inserting a Cast node. Because setup_mappings aliases the graph.input/graph.output ValueInfoProto into value_info_map, this silently changed the model's declared I/O type to the low precision type, violating keep_io_types=True and failing the sanity check: Sanity check failed: Unexpected type in I/O tensor <name>, keep_io_types=True Skip the metadata-only fake-cast for protected network I/O tensors and fall through to insert a real Cast node instead, preserving the declared I/O type while still bridging precision to the low-precision consumer/producer. Add a regression test covering an empty network input feeding a low-precision Concat with keep_io_types=True (fp16 and bf16). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: ajrasane <131806219+ajrasane@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR fixes an ONNX AutoCast bug where ChangesEmpty tensor I/O type preservation fix
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1626 +/- ##
==========================================
- Coverage 77.38% 77.37% -0.01%
==========================================
Files 482 482
Lines 52960 52961 +1
==========================================
- Hits 40985 40981 -4
- Misses 11975 11980 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review — DM the bot to share feedback.
Targeted bug fix for ONNX AutoCast keep_io_types=True with empty I/O tensors. The root-cause analysis (value_info_map aliasing graph.input/graph.output ValueInfoProto, so in-place "fake-cast" mutates declared I/O type) is correct, and the guard cleanly falls through to the real-Cast path only for protected I/O tensors whose target type differs from the original. original_network_io is captured in __init__ from the deepcopied model, so the comparison remains stable across _sanitize_model. Test covers both fp16/bf16 and both type-inference paths and asserts both the I/O-type invariant and the presence of a real Cast node. CHANGELOG updated. Minor gap: no test for the symmetric empty-network-output case, but the fix is symmetric and not worth blocking.
galagam
left a comment
There was a problem hiding this comment.
I'm amazed at the amount of possible corner cases... This time a corner case inside a a WAR for another corner case... Thanks for fixing.
gcunhase
left a comment
There was a problem hiding this comment.
Added a comment, LGTM after that's resolved.
Address review feedback: extract the model construction in test_empty_tensor_network_input_keep_io_types into a model_with_empty_network_input fixture, matching the style of the other model_* fixtures in this test module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: ajrasane <131806219+ajrasane@users.noreply.github.com>
What does this PR do?
Type of change: Bug fix
ONNX AutoCast (used by
modelopt.onnx.quantizationwith--high_precision_dtype fp16/bf16) failed its internal sanity check withUnexpected type in I/O tensor <name>, keep_io_types=True, original type: 1, converted type: 10when a network input or output is an empty tensor (a tensor with a dimension of size 0).Root cause:
PrecisionConverter._add_casttreats empty tensors specially — instead of inserting aCastnode, it "fake-casts" them by retyping theirvalue_infoin place (there is no data to convert). However,setup_mappingsstores the samegraph.input/graph.outputValueInfoProtoobjects invalue_info_map, so retyping an empty tensor that is a network input/output silently changed the model's declared I/O type to the low-precision type — violating thekeep_io_types=Truecontract and tripping the sanity check.Fix (
modelopt/onnx/autocast/precisionconverter.py): skip the metadata-only fake-cast for protected network I/O tensors (whenkeep_io_types=Trueand the target type differs from the original I/O type) and fall through to insert a realCastnode. This preserves the declared I/O type while still bridging precision into the low-precision consumer/producer. Intermediate empty tensors are unchanged and still fake-cast.Usage
Testing
test_empty_tensor_network_input_keep_io_types(fp16 + bf16, both type-inference paths) intests/unit/onnx/autocast/test_precisionconverter.py, asserting network I/O types stay FP32 and a realCastbridges the empty input. Verified it fails before the fix and passes after.CUDA_VISIBLE_DEVICES="" pytest tests/unit/onnx/autocast/→ 202 passed.onnx.checker, runs in ONNX Runtime with correct FP32 I/O, and builds a strongly-typed TensorRT engine.Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: N/A🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests