Skip to content

Conversation

@penguinwu
Copy link
Member

Add the script to dump graph IRs for all the benchmarks

Copy link
Contributor

@bertmaher bertmaher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but do consider my suggestion about warming up rather than disabling the profiling executor.

try:
# disable profiling mode so that the collected graph does not contain
# profiling node
torch._C._jit_set_profiling_mode(False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little wary of this because the non-profiling executor isn't a default mode anymore. I don't know how different the graphs would look but I don't know that it's a good idea to collect stats from a different mode than we run with.

To get rid of the profile nodes, I'd suggest adding a "warmup" before dumping stats:

for _ in range(torch._C._jit_num_profiled_runs()):
  model(*example_inputs)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. I made the default collection to use the default setting (i.e., profile executor enabled and warmed up). Bu I added additional options to dump graphs as inlined_graph and also dump graph w/o profiling executor. Sometimes we need to see the graph before profiling executor to check if TS IR level optimizations can handle them (e.g., for symbolic shape analysis or alias analysis).

usage: collect_graph_ir.py [-h] [--benchmark BENCHMARK] [--no_profiling] [--inlined_graph]

dump last_executed graph for all benchmarks with JIT implementation

optional arguments:
  -h, --help            show this help message and exit
  --benchmark BENCHMARK, -b BENCHMARK
                        dump graph for <BENCHMARK>
  --no_profiling        dump last_executed graphs w/o profiling executor
  --inlined_graph       dump graphs dumped by module.inlined_graph

Copy link
Contributor

@wconstab wconstab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm other than comments from Bert. Thanks for adding this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants