-
Notifications
You must be signed in to change notification settings - Fork 800
Enable test_ft_map_basic and test_ft_map_dynshape tests #16666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16666
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 Cancelled Jobs, 6 Unrelated FailuresAs of commit dd7785c with merge base 9cdc558 ( CANCELLED JOBS - The following jobs were cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@mergennachin has exported this pull request. If you are a Meta employee, you can view the originating Diff in D90890790. |
This PR needs a
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enables two previously skipped tests for higher-order map operations by migrating them from the deprecated exir.CaptureConfig API to the modern torch.export.export() pipeline. The tests were previously disabled with "Emitter is not ready yet" but the actual issue was that they used deprecated APIs.
Changes:
- Removed
@skipUnlessdecorators fromtest_ft_map_basicandtest_ft_map_dynshape - Migrated both tests from
maketest()helper to manualtorch.export.export() -> to_edge() -> to_executorch()pipeline - Added comprehensive docstrings explaining map operation behavior and limitations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ), | ||
| )(self) | ||
| """Test FTMapBasic model through the modern torch.export API.""" | ||
| from executorch.exir import EdgeCompileConfig, to_edge |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement from executorch.exir import EdgeCompileConfig, to_edge is redundant since EdgeCompileConfig is already imported at the module level (line 26). Consider using the existing module-level import and only importing to_edge locally, or move both imports to the top of the file for consistency with other imports in this test file.
| from executorch.exir import EdgeCompileConfig, to_edge | |
| from executorch.exir import to_edge |
| ), | ||
| )(self) | ||
| """Test FTMapBasic model through the modern torch.export API.""" | ||
| from executorch.exir import EdgeCompileConfig, to_edge |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement from executorch.exir import EdgeCompileConfig, to_edge is redundant since EdgeCompileConfig is already imported at the module level (line 26). Consider using the existing module-level import and only importing to_edge locally, or move both imports to the top of the file for consistency with other imports in this test file.
| from executorch.exir import EdgeCompileConfig, to_edge | |
| from executorch.exir import to_edge |
0a27a0b to
34536c9
Compare
|
@mergennachin has imported this pull request. If you are a Meta employee, you can view this in D90890790. |
Summary: These tests were skipped with "Emitter is not ready yet" but the issue was that they were using the deprecated exir.CaptureConfig API. Rewrote both tests to use the modern torch.export.export() -> to_edge() -> to_executorch() pipeline. Note: The higher-order map operation specializes on the iteration dimension at export time, so varying batch sizes are not supported. The tests now verify that map-based models can be correctly exported and executed through the ExecuTorch pipeline. Authored with Claude Code. Reviewed By: rascani Differential Revision: D90890790 Pulled By: mergennachin
34536c9 to
dd7785c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ), | ||
| )(self) | ||
| """Test FTMapBasic model through the modern torch.export API.""" | ||
| from executorch.exir import EdgeCompileConfig, to_edge |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EdgeCompileConfig is already imported at the module level (line 26). Only to_edge needs to be imported here since it's not available at the module level.
| from executorch.exir import EdgeCompileConfig, to_edge | |
| from executorch.exir import to_edge |
| # Test execution matches eager mode | ||
| eager_output = model(*inputs) | ||
| et_output = executorch_module.forward(list(inputs))[0] | ||
|
|
||
| # Compare outputs | ||
| torch.testing.assert_close( | ||
| et_output, | ||
| eager_output, | ||
| rtol=1e-5, | ||
| atol=1e-8, | ||
| msg="ExecuTorch output doesn't match eager output", | ||
| ) |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test only validates a single execution with one set of inputs. The previous maketest approach ran 10 iterations with different random inputs (niter=10) to catch edge cases and ensure consistent behavior. Consider adding a loop to test multiple random input sets for more robust validation.
| that the map-based model can be exported and executed correctly through the | ||
| ExecuTorch pipeline using the modern torch.export.export() API. | ||
| """ | ||
| from executorch.exir import EdgeCompileConfig, to_edge |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EdgeCompileConfig is already imported at the module level (line 26). Only to_edge needs to be imported here since it's not available at the module level.
| # Test execution matches eager mode | ||
| eager_output = model(*inputs) | ||
| et_output = executorch_module.forward(list(inputs))[0] | ||
|
|
||
| # Compare outputs | ||
| torch.testing.assert_close( | ||
| et_output, | ||
| eager_output, | ||
| rtol=1e-5, | ||
| atol=1e-8, | ||
| msg="ExecuTorch output doesn't match eager output", | ||
| ) |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test only validates a single execution with one set of inputs. The previous maketest approach ran 10 iterations with different random inputs (niter=10) to catch edge cases and ensure consistent behavior. Consider adding a loop to test multiple random input sets for more robust validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Summary:
These tests were skipped with "Emitter is not ready yet" but the issue was
that they were using the deprecated exir.CaptureConfig API. Rewrote both tests
to use the modern torch.export.export() -> to_edge() -> to_executorch() pipeline.
Note: The higher-order map operation specializes on the iteration dimension at
export time, so varying batch sizes are not supported. The tests now verify that
map-based models can be correctly exported and executed through the ExecuTorch
pipeline.
Authored with Claude Code.
Differential Revision: D90890790