fix: validate cached ONNX before reuse to prevent empty-graph TensorRT build crash#34
Open
forkni wants to merge 1 commit into
Open
fix: validate cached ONNX before reuse to prevent empty-graph TensorRT build crash#34forkni wants to merge 1 commit into
forkni wants to merge 1 commit into
Conversation
…T build crash An interrupted/failed ONNX export leaves a syntactically-valid-but-empty protobuf at onnx_path. The export cache check was existence-only, so the empty file was reused, feeding a 0-node/opset-less graph into polygraphy fold_constants -> ORT shape inference bails on opset<7 -> returns None -> get_num_nodes(None) crashes as "'NoneType' object has no attribute 'graph'". builder.py: add _onnx_cache_valid() (size>0, >=1 node, opset>=7, loaded structure-only) and gate the export cache-hit on it; delete + re-export a bad cache entry. models.py: guard Optimizer.fold_constants() against a 0-node graph. This is the single call site shared by both BaseModel.optimize() and CLIP's optimize() override (which calls fold_constants() directly, bypassing BaseModel.optimize()), so placing the guard here -- rather than only in optimize() -- covers every model type (CLIP/UNet/VAE encoder/decoder). Co-Authored-By: Claude Sonnet 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A user hit a crash during TensorRT engine build:
Root cause: a prior build was interrupted mid-
torch.onnx.export()(OOM, crash, manualkill).
torch.onnx.export()writes directly to the finalonnx_pathwith no temp-file-then-rename, so the interrupted run left a syntactically-valid-but-empty (or truncated) protobuf on
disk.
EngineBuilder.build()'s cache check is existence-only (os.path.exists(onnx_path)), sothe next run silently reused the corrupt file instead of re-exporting. That empty/0-node graph
then reached
Optimizer.fold_constants()→ polygraphy'sfold_constants→ ONNX Runtime'ssymbolic shape inference, which refuses opset < 7 graphs and returns
None— crashingget_num_nodes(None)with the opaqueAttributeErrorabove, several layers away from theactual cause.
Fix (two layers, so every model type is covered)
builder.py: add_onnx_cache_valid(path)— loads the ONNX structure-only (no externalweight data, so it's cheap even for large models), and rejects it if the file is empty, has
0 graph nodes, or has no opset ≥ 7. Gate the export cache-hit on this check instead of on
os.path.exists()alone; if a cached file fails validation, delete it and re-export insteadof silently reusing (or crashing on) garbage.
models.py: guardOptimizer.fold_constants()itself against a 0-node graph, rather thanonly guarding
BaseModel.optimize().CLIP.optimize()overridesBaseModel.optimize()entirely and calls
fold_constants()directly, so a guard placed only inBaseModel.optimize()wouldn't catch a corrupt CLIP ONNX. Placing it infold_constants()covers CLIP, UNet, VAE encoder, and VAE decoder from the one shared call site.
With this fix, an interrupted-export leftover is detected and transparently re-exported instead
of crashing on the next run.
Test plan
ast.parse()on both modified files (syntax check in this sandbox)merged in fix: validate cached ONNX before reuse to prevent empty-graph TensorRT build crash forkni/StreamDiffusion#16 and fix: core-pipeline crash fixes + effective-python audit (9 commits) #17) resolves the reported crash
.onnx, rerun build, confirm re-export +no crash) — requires a CUDA/TensorRT machine, not runnable in this environment; the
reporting user's original repro trace is included above as evidence of the failure mode.
🤖 Generated with Claude Code