Skip to content

Commit 62ef2af

Browse files
Prevent edge_transform_passes from running twice during recipe-based lowering (pytorch#23005)
### Summary After a recent change, the `edge_transform_passes` were also run by the `EdgeProgramManagerTransformStage` as well as the `EdgeTransformAndLowerStage`, resulting in a double application. This PR removes them from the `EdgeProgramManagerTransformStage`. ### Test plan `pytest export/tests/test_export_session.py`
1 parent 888c94f commit 62ef2af

3 files changed

Lines changed: 30 additions & 38 deletions

File tree

‎export/recipe.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,12 @@ class LoweringRecipe:
207207
per-method partitioner lists. Use the dict form when
208208
backends need per-method compile specs.
209209
edge_transform_passes: Optional list of callables that take (method_name: str, exported_program: ExportedProgram)
210-
and return either List[PassType] or PassManager to be applied during edge lowering.
210+
and return either List[PassType] or PassManager. Applied during
211+
TO_EDGE_TRANSFORM_AND_LOWER as per-ExportedProgram graph-module passes.
211212
edge_manager_transform_passes: Optional list of callables that take EdgeProgramManager as argument
212-
and return passes to be applied. Applied sequentially after TO_EDGE stage.
213+
and return passes to be applied. Applied sequentially by
214+
EDGE_PROGRAM_MANAGER_TRANSFORM, which runs after
215+
TO_EDGE_TRANSFORM_AND_LOWER in the default pipeline.
213216
edge_compile_config: Optional edge compilation configuration
214217
pre_partitioning_callback: Optional callable invoked just before partitioning with
215218
`(partitioners, programs)` arguments.

‎export/stages.py‎

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -734,14 +734,6 @@ class EdgeProgramManagerTransformStage(Stage):
734734

735735
def __init__(
736736
self,
737-
edge_transform_passes: (
738-
None
739-
| List[
740-
Callable[
741-
[str, ExportedProgram], List[PassType] | GraphModulePassManager
742-
]
743-
]
744-
) = None,
745737
edge_manager_transform_passes: (
746738
None
747739
| List[
@@ -759,7 +751,6 @@ def __init__(
759751
backends to control pass ordering and dependencies.
760752
"""
761753
super().__init__()
762-
self._edge_transform_passes = edge_transform_passes or []
763754
self._edge_manager_transform_passes = edge_manager_transform_passes or []
764755

765756
@classmethod
@@ -770,7 +761,6 @@ def from_recipe(
770761
return cls()
771762

772763
return cls(
773-
edge_transform_passes=lowering_recipe.edge_transform_passes,
774764
edge_manager_transform_passes=lowering_recipe.edge_manager_transform_passes,
775765
)
776766

@@ -803,35 +793,10 @@ def run(self, artifact: PipelineArtifact) -> None:
803793
f"Expected EdgeProgramManager but got {type(edge_program_manager)}"
804794
)
805795

806-
if not self._edge_transform_passes and not self._edge_manager_transform_passes:
796+
if not self._edge_manager_transform_passes:
807797
self._artifact = artifact
808798
return
809799

810-
# Detect if any callable returns PassManager
811-
pass_manager = None
812-
transform_passes = defaultdict(list)
813-
for method_name in edge_program_manager.methods:
814-
# Resolve transform passes if it's a callable
815-
ep = edge_program_manager.exported_program(method_name)
816-
for pass_callable in self._edge_transform_passes or []:
817-
if not callable(pass_callable):
818-
raise ValueError(
819-
"Transform passes must be a callable that resolves to passes"
820-
)
821-
passes = pass_callable(method_name, ep)
822-
if isinstance(passes, GraphModulePassManager):
823-
pass_manager = passes
824-
break
825-
else:
826-
transform_passes[method_name].extend(passes)
827-
if pass_manager:
828-
break
829-
830-
# See EdgeTransformAndLowerStage.run.
831-
final_passes = pass_manager or _drop_empty(transform_passes) or None
832-
if final_passes is not None:
833-
edge_program_manager = edge_program_manager.transform(final_passes)
834-
835800
# Run edge manager transform passes
836801
for pass_callable in self._edge_manager_transform_passes:
837802
passes = pass_callable(edge_program_manager)

‎export/tests/test_export_session.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,30 @@ def test_no_transform_passes_means_no_stage(self) -> None:
757757
StageType.EDGE_PROGRAM_MANAGER_TRANSFORM, session._get_default_pipeline()
758758
)
759759

760+
def test_edge_transform_passes_not_duplicated_in_default_pipeline(self) -> None:
761+
# Before the fix, from_recipe() gave edge_transform_passes to
762+
# EDGE_PROGRAM_MANAGER_TRANSFORM as well as TO_EDGE_TRANSFORM_AND_LOWER,
763+
# so they ran twice. Verify the stage no longer holds them at all.
764+
from executorch.export.stages import EdgeProgramManagerTransformStage
765+
766+
edge_pass = Mock()
767+
epm_pass = Mock()
768+
769+
stage = EdgeProgramManagerTransformStage.from_recipe(
770+
LoweringRecipe(
771+
edge_transform_passes=[edge_pass],
772+
edge_manager_transform_passes=[epm_pass],
773+
)
774+
)
775+
776+
# The stage must only know about edge_manager_transform_passes.
777+
self.assertEqual(stage._edge_manager_transform_passes, [epm_pass])
778+
self.assertFalse(
779+
hasattr(stage, "_edge_transform_passes"),
780+
"EdgeProgramManagerTransformStage must not hold edge_transform_passes "
781+
"because TO_EDGE_TRANSFORM_AND_LOWER already applies them.",
782+
)
783+
760784
def test_example_inputs_required_for_nn_module(self) -> None:
761785
"""Test that example_inputs are required for nn.Module."""
762786
with self.assertRaises(ValueError) as cm:

0 commit comments

Comments
 (0)