Repository navigation
fix(dynamo): keep the exported signature of models with complex values - #4891
Conversation
shoumikhin
left a comment
There was a problem hiding this comment.
Not blocking, for a follow-up: models with a complex input or output still cannot be called by keyword. At the end of _insert_complex_io_adapters, the codegen is rebuilt from the placeholder names (arg0_1, ...), which replaces the one this PR restores. Its comment says setting the codegen before partitioning crashes, which is no longer true with this change. Removing that rebuild fixed keyword calls for complex inputs and outputs in my runs, and the complex test files still passed.
| # (keyword inputs), which expects the graph to return flat leaves | ||
| if isinstance(original_codegen, torch.fx.graph._PyTreeCodeGen): | ||
| output = gm.graph.output_node() | ||
| output.args = (tuple(torch.utils._pytree.tree_leaves(output.args[0])),) |
There was a problem hiding this comment.
This breaks models whose output is a namedtuple or a dataclass registered with register_dataclass. make_fx records building that object as one call_function node, so output.args[0] is a single Node and tree_leaves returns one leaf. The restored out_spec expects two, so every call fails, also by position:
ValueError: treespec.unflatten(leaves): `leaves` has length 1 but the spec refers to a pytree that holds 2 items
On main the same model works when called by position. To reproduce, take the model from the new test and return NT(y, y * 2) with NT = collections.namedtuple("NT", ["a", "b"]).
An option that worked for me, without flattening: give gm a plain torch.fx.graph.CodeGen() only while decompose_complex_in_graph runs, so the retrace returns the flat graph outputs. Then put original_codegen back on gm and on the returned module. With that, namedtuple and dataclass outputs work again, and single, nested and dict outputs still work by position, by keyword and through save. A namedtuple case in the new test would keep this covered.
Three places lost a complex model's exported signature or a complex output: - complex_decomposition_adapter retraces the graph with make_fx, which returns a plain codegen taking positional flat args, so the compiled module rejected its own keyword inputs (Llama 4's complex rotary embedding: forward() got an unexpected keyword argument 'input_ids'). Trace with a plain codegen, so the retrace returns the graph's flat outputs, and put the exported program's codegen back afterwards. Restoring it on a retraced graph that returns the user's output structure breaks namedtuple and register_dataclass outputs, which make_fx records as built by one node. - _insert_complex_io_adapters rebuilt the codegen from the graph's placeholder names (arg0_1, x_unpacked_complex), so models with a complex input or output could only be called by position. Keep the exported program's codegen when it survived lowering, and rebuild one only if it was lost. - The output boundary turned a block's output back into a complex tensor only when the output was the block call itself. A block with several outputs returns each through a getitem, so such a complex output came back as a real [..., 2] tensor. Treat a getitem of a block as its output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2a5cd87 to
5dbd7f4
Compare
shoumikhin
left a comment
There was a problem hiding this comment.
Tested on A100: the new tests fail before this PR and pass with it, and the complex test files pass, also with #4896 merged in. Looks good to me. Two small things inline, neither blocks.
| # A block with several outputs returns this one through a getitem | ||
| block = ( | ||
| src.args[0] | ||
| if src.op == "call_function" and src.target is operator.getitem |
There was a problem hiding this comment.
This also wraps outputs that are already complex. Example: return torch.linalg.eig(x + 1) on the legacy path. eig runs in a PyTorch block, and the call now fails with "view_as_complex is not supported for complex tensors". It worked before this PR. A single-output eigvals already failed the same way before. The boundary should only convert outputs that were packed as real, not outputs that are already complex. That would fix both.
| torch.testing.assert_close(out(x=x), model(x)) | ||
|
|
||
|
|
||
| def test_interior_complex_region_keeps_a_namedtuple_output(): |
There was a problem hiding this comment.
This test still passes if I remove the plain CodeGen swap before the retrace. A namedtuple is also a tuple, so it gets rebuilt by accident. A dataclass output fails without the swap, so it would protect the swap better.
#4891) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Models with complex values lost their exported signature or a complex output in three places. The review pointed out the first regression and the second limitation; this PR now fixes all three.
complex_decomposition_adapterretraces the graph withmake_fx. That returns a plainCodeGentaking positional flat arguments, so the compiled module rejected its own keyword inputs. Llama 4's complex rotary embedding hits this:register_dataclassoutputs:make_fxrecords building them as one node, so the restored codegen found one leaf instead of two._insert_complex_io_adapters, the codegen was rebuilt from the graph's placeholder names (arg0_1,x_unpacked_complex). Such models could only be called by position.view_as_complexonly when the output was the block call itself. A block with several outputs returns each through agetitem.{"prod": a * b, "energy": ...},prodcame back as a real[..., 2]tensor, on both complex paths. This predates the PR.getitemof a block now counts as that block's output.Export → compile → call on both complex paths:
prodreturned realTests
test_interior_complex_region_keeps_a_namedtuple_output(the adapter, by position and by keyword);test_complex_input_complex_output_by_keyword[legacy|decomp];test_complex_output_of_a_multi_output_block[legacy|decomp].test_interior_complex_region_keeps_exported_signature:pytest lowering/test_complex_decomposition_adapter.py lowering/test_complex_rewrite.py hlo/test_complex_ops.py hlo/test_complex_graph_break.py hlo/test_rope_embedding.py: 323 passed (the 318 of the first version, plus the 5 new ones).models/test_model_refit.py -k complex: 3 passed.trl-internal-testing/tiny-Llama4ForCausalLMruns and matches eager (cosine 1.0): one TensorRT engine, nothing left to PyTorch.🤖 Generated with Claude Code