Fix ONNX export of channels_last_3d pre-decoder models; fail fast on requested export failures#92
Merged
Conversation
…port failure The eval-input layout match introduced with the AMP training work calls contiguous(memory_format=channels_last_3d) inside PreDecoderMemoryEvalModule.forward. The surface inference path converts every model to channels_last_3d before export, so the traced graph contains a memory-format op the legacy exporter cannot lower and torch.onnx.export(..., dynamo=False) fails with "onnx memory_format support is not implemented". Skip the layout match during export: ONNX has no memory-format concept, so the artifact is value-identical. Also stop silently downgrading to PyTorch when ONNX_WORKFLOW=1/2 was explicitly requested and the export fails: broadcast the failure to all ranks (so multi-GPU runs exit promptly instead of blocking in the next collective) and raise, making local_run.sh exit nonzero. INT8 quantization failure in the ablation path now falls back to the FP32 ONNX like the LER path instead of aborting the run; FP8 stays fail-fast. Add onnx to the public inference requirements (the legacy exporter needs it to serialize the requested artifact) and add a CI-collected regression test that exports a channels_last_3d wrapper and checks the ONNX output against eager. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cketcham2333
approved these changes
Jul 22, 2026
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.
Problem
Since #89, exporting the surface pre-decoder to ONNX (
ONNX_WORKFLOW=1/2) fails for every SafeTensors model (Fast and Accurate alike):The eval-input layout match (
match_input_to_model_memory_format) runs insidePreDecoderMemoryEvalModule.forward, and the inference path unconditionally converts the model tochannels_last_3dbefore export — so tracing bakesaten::contiguous(memory_format=channels_last_3d)into the graph, which the legacy (dynamo=False) exporter cannot lower. Worse, the failure was swallowed: the run fell back to PyTorch and exited 0, so automation believed the requested artifact had been produced. (Color-code workflows are unaffected: nochannels_last_3dconversion and no export on that path.)Fix
training/precision.py:match_input_to_model_memory_format()is now a no-op during ONNX export (torch.onnx.is_in_onnx_export()), covering all call sites at once. The op is layout-only and ONNX has no memory-format concept, so the exported graph is value-identical — verified in the new test by comparing the ONNX reference-evaluator output against eager.evaluation/logical_error_rate.py,evaluation/failure_analysis.py: a failed explicitly-requested export now raises (nonzero exit) instead of silently falling back to PyTorch. Rank 0 broadcasts the failure so multi-GPU runs exit promptly on every rank instead of blocking in the next collective. INT8 quantization failure in the ablation path now falls back to the FP32 ONNX (mirroring the LER path and the README); FP8 stays fail-fast.requirements_public_inference.txt: addonnx— the legacy exporter requires it at serialization time, soONNX_WORKFLOW=1could not produce an artifact without it. This also lets CI run the new regression test.unittest discovercollects it in CI); the ablation export-failure test now asserts the fail-fast contract; unit tests for the shared fail-fast helper.Testing
mainwith the exact error above and passes with the fix.test_precision.py,test_failure_analysis.py,test_tensorrt_fallback.py: 95 passed, 1 skipped (tensorrt absent), including CI-styleunittest discover;yapf --diffclean.channels_last_3dConv3d wrapper fails, guarded export succeeds, eager outputs bit-identical with and without the layout conversion.🤖 Generated with Claude Code