Skip to content

Add FX-based profiler#256

Merged
jansel merged 5 commits intopytorch:masterfrom
jansel:fxprof202102
Feb 26, 2021
Merged

Add FX-based profiler#256
jansel merged 5 commits intopytorch:masterfrom
jansel:fxprof202102

Conversation

@jansel
Copy link
Copy Markdown
Contributor

@jansel jansel commented Feb 14, 2021

Perhaps others would find this useful. Results as of last week.

@jansel jansel requested a review from wconstab February 25, 2021 03:20
@jansel
Copy link
Copy Markdown
Contributor Author

jansel commented Feb 25, 2021

@wconstab you think it makes sense to merge this? Or just leave it as a standalone thing.

@wconstab
Copy link
Copy Markdown
Contributor

@jansel yea I'd definitely merge it in. I will review now. I'm wondering if this is something we'd want to run every nightly or at some interval, or just for manual use.


def run_model(model_class, model_path, device):
m = model_class(device=device)
def test_fx_profile(self):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this is a correctness test rather than something you're using to gather data. Did you confirm if this gets invoked in CI as you wrote it? I think we run all tests from test.py.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct, this is just a correctness test. I verified it locally, I did not check the CI though.

Copy link
Copy Markdown
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, and I confirmed there is one new test (157 on your branch vs 156 master) being run in CI although the test names aren't logged.

Depending on what people want to do with this we can talk about automating it in CI to collect stats or whatever. But seems good/useful as-is so please merge.

@jansel jansel merged commit 9f42a86 into pytorch:master Feb 26, 2021
@jansel jansel deleted the fxprof202102 branch February 26, 2021 02:38
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.

3 participants