[AOTI Minifier] Save EP instead of graphs #141159

yushangdi · 2024-11-20T20:19:53Z

Summary:
repro.py can have nested graph modules, e.g.

class Repro(torch.nn.Module):
    def __init__(self) -> None:
        super().__init__()
        self.true_graph_0 = GraphModule()



    def forward(self):
        true_graph_0 = self.true_graph_0
        return (true_graph_0,)

So dumping the string doesn’t always work.

So,

we use exported program in repro.py instead
we still dump the graph module string, but only put it in comments

We also added two flags to minifier_launcher.py

minifier-export-mode: whether strict or non-strict export is used in the minifier
skip-export-error: intermediate graphs that cannot be exported will be skipped.

Test Plan:

buck2 run  fbcode//caffe2/test/inductor:minifier_utils_cpu  -- -r string
python test/inductor/test_minifier.py

Differential Revision: D66175257

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang @aakhundov

pytorch-bot · 2024-11-20T20:19:57Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/141159

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

[DomainsOnly] Jobs fail with GLIBC version not found

✅ No Failures

As of commit f148683 with merge base 149677e ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot · 2024-11-20T20:20:13Z

This pull request was exported from Phabricator. Differential Revision: D66175257

henrylhtsang · 2024-11-21T18:49:11Z

buck2 run fbcode//caffe2/test/inductor:minifier_utils_cpu -- -r string

nit: generally its a good idea to keep fbcode and oss separated. But I find it hard as well

henrylhtsang · 2024-11-21T18:58:32Z

torch/_dynamo/repro/aoti.py

+    elif gm is None:
+        gm = exported_program.module()
+
+    # save a graph preview using gm


so just to be clear, gm is just for string. exported_program is the thing that actually matters?

so just to be clear, gm is just for string. exported_program is the thing that actually matters?

Yes, but note that we also have exported_program = torch.export.export(gm, args, strict=strict) three lines above. So if the input is gm, the exported_program is generated from gm, so in some sense, gm also matters.

henrylhtsang · 2024-11-21T18:59:46Z

torch/_dynamo/repro/aoti.py

        save_dir=subdir,
        command="minify",
-        options=options,
+        config_patches=options,


so it will just use strict = False?

so it will just use strict = False?

By default yes, but you can set minifier_export_mode="dynamo" kwarg in the run_repro() function in minifier_launcher to change it to use strict=True.

I'll update the doc to include this later (the doc is a bit annoying to edit because the doc build can fail if not edited carefully, so I prefer to update the doc in a separate PR).

henrylhtsang · 2024-11-21T19:00:26Z

torch/_dynamo/repro/aoti.py

    command="run",
    accuracy=None,
    check_str=None,
+    module_in_comment=False,


I guess you will use it in the future?

I guess you will use it in the future?

yes! I think this PR is getting a little long already.

Basically, it's for not having the module in comment in minifier_launcher.py, since the module can be quite large.

henrylhtsang · 2024-11-21T19:04:21Z

torch/_dynamo/repro/aoti.py

+                return None
        raise AOTIMinifierError(e) from e
-
+    return None


not sure what this does but okay

not sure what this does but okay

yeah we should never reach here. I'll add a comment.

Summary: `repro.py` can have nested graph modules, e.g. ``` class Repro(torch.nn.Module): def __init__(self) -> None: super().__init__() self.true_graph_0 = GraphModule() def forward(self): true_graph_0 = self.true_graph_0 return (true_graph_0,) ``` So dumping the string doesn’t always work. So, 1) we use exported program in repro.py instead 2) we still dump the graph module string, but only put it in comments We also added two flags to `minifier_launcher.py` - `minifier-export-mode`: whether strict or non-strict export is used in the minifier - `skip-export-error`: intermediate graphs that cannot be exported will be skipped. Test Plan: ``` buck2 run fbcode//caffe2/test/inductor:minifier_utils_cpu -- -r string python test/inductor/test_minifier.py ``` Reviewed By: henrylhtsang Differential Revision: D66175257

facebook-github-bot · 2024-11-21T19:50:55Z

This pull request was exported from Phabricator. Differential Revision: D66175257

Summary: `repro.py` can have nested graph modules, e.g. ``` class Repro(torch.nn.Module): def __init__(self) -> None: super().__init__() self.true_graph_0 = GraphModule() def forward(self): true_graph_0 = self.true_graph_0 return (true_graph_0,) ``` So dumping the string doesn’t always work. So, 1) we use exported program in repro.py instead 2) we still dump the graph module string, but only put it in comments We also added two flags to `minifier_launcher.py` - `minifier-export-mode`: whether strict or non-strict export is used in the minifier - `skip-export-error`: intermediate graphs that cannot be exported will be skipped. Test Plan: ``` buck2 run fbcode//caffe2/test/inductor:minifier_utils_cpu -- -r string python test/inductor/test_minifier.py ``` Reviewed By: henrylhtsang Differential Revision: D66175257

facebook-github-bot · 2024-11-21T20:44:58Z

This pull request was exported from Phabricator. Differential Revision: D66175257

Summary: `repro.py` can have nested graph modules, e.g. ``` class Repro(torch.nn.Module): def __init__(self) -> None: super().__init__() self.true_graph_0 = GraphModule() def forward(self): true_graph_0 = self.true_graph_0 return (true_graph_0,) ``` So dumping the string doesn’t always work. So, 1) we use exported program in repro.py instead 2) we still dump the graph module string, but only put it in comments We also added two flags to `minifier_launcher.py` - `minifier-export-mode`: whether strict or non-strict export is used in the minifier - `skip-export-error`: intermediate graphs that cannot be exported will be skipped. Test Plan: ``` buck2 run fbcode//caffe2/test/inductor:minifier_utils_cpu -- -r string python test/inductor/test_minifier.py ``` Reviewed By: henrylhtsang Differential Revision: D66175257

facebook-github-bot · 2024-11-21T20:54:08Z

This pull request was exported from Phabricator. Differential Revision: D66175257

facebook-github-bot · 2024-11-22T01:43:47Z

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorchmergebot · 2024-11-22T01:45:28Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Summary: `repro.py` can have nested graph modules, e.g. ``` class Repro(torch.nn.Module): def __init__(self) -> None: super().__init__() self.true_graph_0 = GraphModule() def forward(self): true_graph_0 = self.true_graph_0 return (true_graph_0,) ``` So dumping the string doesn’t always work. So, 1) we use exported program in repro.py instead 2) we still dump the graph module string, but only put it in comments We also added two flags to `minifier_launcher.py` - `minifier-export-mode`: whether strict or non-strict export is used in the minifier - `skip-export-error`: intermediate graphs that cannot be exported will be skipped. Test Plan: ``` buck2 run fbcode//caffe2/test/inductor:minifier_utils_cpu -- -r string python test/inductor/test_minifier.py ``` Differential Revision: D66175257 Pull Request resolved: pytorch#141159 Approved by: https://github.com/henrylhtsang

pytorch-bot bot added ciflow/inductor module: dynamo module: inductor labels Nov 20, 2024

facebook-github-bot added the fb-exported label Nov 20, 2024

yushangdi added the topic: not user facing topic category label Nov 20, 2024

henrylhtsang reviewed Nov 21, 2024

View reviewed changes

henrylhtsang approved these changes Nov 21, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 21, 2024

facebook-github-bot force-pushed the export-D66175257 branch from 6482f4f to cde00ca Compare November 21, 2024 19:50

facebook-github-bot force-pushed the export-D66175257 branch from cde00ca to 018874e Compare November 21, 2024 20:44

facebook-github-bot force-pushed the export-D66175257 branch from 018874e to f148683 Compare November 21, 2024 20:53

pytorchmergebot added the merging label Nov 22, 2024

pytorchmergebot added the Merged label Nov 22, 2024

pytorchmergebot closed this in f28bac7 Nov 22, 2024

pytorchmergebot removed the merging label Nov 22, 2024

github-actions bot deleted the export-D66175257 branch December 22, 2024 02:11

[AOTI Minifier] Save EP instead of graphs #141159

[AOTI Minifier] Save EP instead of graphs #141159

Uh oh!

Conversation

yushangdi commented Nov 20, 2024 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/141159

❗ 1 Active SEVs

✅ No Failures

Uh oh!

facebook-github-bot commented Nov 20, 2024

Uh oh!

henrylhtsang commented Nov 21, 2024

Uh oh!

henrylhtsang Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

yushangdi Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

henrylhtsang Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

yushangdi Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

henrylhtsang Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

yushangdi Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

henrylhtsang Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

yushangdi Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 21, 2024

Uh oh!

facebook-github-bot commented Nov 21, 2024

Uh oh!

facebook-github-bot commented Nov 21, 2024

Uh oh!

facebook-github-bot commented Nov 22, 2024

Uh oh!

pytorchmergebot commented Nov 22, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yushangdi commented Nov 20, 2024 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Nov 20, 2024 •

edited

Loading

yushangdi Nov 21, 2024 •

edited

Loading