fix(dynamo): stop unpacking complex inputs before the refit output check - #4583
Open
shoumikhin wants to merge 2 commits into
Open
fix(dynamo): stop unpacking complex inputs before the refit output check#4583shoumikhin wants to merge 2 commits into
shoumikhin wants to merge 2 commits into
Conversation
Three refit tests fail on main: test_complex_buffer_refit test_dual_complex_buffer_refit test_complex_buffer_with_real_param_refit TypeError: view_as_real is only supported for complex tensors After refitting, `refit_module_weights(verify_output=True)` runs the lowered PyTorch module and the compiled module on the same inputs and compares the results. For a model with a complex input it first converted that input to a real tensor of shape (..., 2), because the lowering pass used to rewrite the complex placeholder to a real one. It does not do that any more. The placeholder stays complex and the graph starts with its own `view_as_real`, so the reference module was being handed a real tensor and then asked to unpack it again, which is what raises. Pass the caller's inputs to both modules, which is what the non-complex path already did, so the two paths become one.
The repository lint job runs `black --check .` across the whole tree, so any file that does not match the formatter fails CI for every open pull request, not only the one that touched it. `tests/py/dynamo/conversion/test_cumsum_aten.py` is currently not black-conformant on main, which turns the Python Linting check red here. Reformat that one file with black. This is a formatting-only change: two statements that fit on a single line are un-wrapped. No test logic changes. Verified by running `black --check .` on the full tree: all files pass.
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.
What is broken
Three tests are red on
main:test_complex_buffer_refittest_dual_complex_buffer_refittest_complex_buffer_with_real_param_refitWhy
After refitting,
refit_module_weights(verify_output=True)runs the loweredPyTorch module and the compiled module on the same inputs and compares the
results. When an input is complex it first converted that input to a real tensor
of shape
(..., 2), because the lowering pass used to rewrite a complexplaceholder into a real one.
That is no longer what the pass does. The placeholder stays complex and the
graph does the unpacking itself:
So the reference module got a real tensor and was then asked to unpack it a
second time, which is what raises.
Fix
Give both modules the inputs the caller passed. That is what the non-complex
path already did, so the special case goes away and the two paths become one.
Tested
On an H100 with the three tests above: they fail before this change and pass
after it. The whole file passes, 7 passed and 12 skipped, the skips being the
ones
mainalready skips in this environment.