Skip to content
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

[Reporting Tool] Modular report classes for saving non-Thunder-specific repro scripts #1700

Open
kiya00 opened this issue Jan 27, 2025 · 0 comments · May be fixed by #1707
Open

[Reporting Tool] Modular report classes for saving non-Thunder-specific repro scripts #1700

kiya00 opened this issue Jan 27, 2025 · 0 comments · May be fixed by #1707
Assignees

Comments

@kiya00
Copy link
Collaborator

kiya00 commented Jan 27, 2025

For the next PR, I think it'd be useful to create an FXReport object. What I'm thinking is:

  • Create a new function called fx_report (not Thunder-specific)
  • Return a FXReport object from fx_report
  • The FXReport object has a sequence of FXGraphReport objects, one per fx graph in the original program
  • Each FXGraphReport has a method to write a reproducer script
    • The method should be able to easily select whether the reproducer uses PyTorch eager, torch.compile, or thunderfx (with some supported options)
    • For total control, the method should allow writing an arbitrary executor string and allow for extending what's imported with arbitrary import strings, in case someone wants to use a very special version of thunderfx or torch.compile or PyTorch eager
  • Each FXGraphReport should allow its FXGraph's inputs to be queried (they can just be FakeTensors for now)

So someone could do something like:

report: FXReport = fx_report(foo)

fxgr: FXGraphReport
for fxgr in report.graph_reports:
  print(fxgr.inputs)
  fxgr.write_eager_reproducer(filename0)
  fxgr.write_torchcompile_reproducer(filename1)
  fxgr.write_thunder_reproducer(filename2)
  fxgr.write_reproducer(filename3, executor_string="my_crazy_executor", import_strings=("import my_crazy_module", "from my_crazy_module import my_crazy_executor"))

And that would generate four reproducers for each of the FX graphs in the program, each one using the different requested executor.

Writing the reproducers directly like this may not be super interesting in isolation, but once that's working we can add more of the data that thunderfx_report makes available, and that will become really interesting really fast. It would also be great if the existing reporting functionality, like thunderfx_report could be updated to use FXGraphReport and FXReport objects internally for consistency. Using modular Python objects for the report data and analysis should make it easier for us to develop and for others to extend.

What are your thoughts, @kiya00?

Originally posted by @mruberry in #1636 (comment)

@kiya00 kiya00 self-assigned this Jan 27, 2025
@kiya00 kiya00 mentioned this issue Jan 27, 2025
4 tasks
kiya00 added a commit that referenced this issue Jan 28, 2025
kiya00 added a commit that referenced this issue Jan 28, 2025
@kiya00 kiya00 linked a pull request Jan 28, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant