Skip to content

Improve PyTorch profiler flop computation formulas#51377

Closed
xuzhao9 wants to merge 1 commit intopytorch:masterfrom
xuzhao9:xzhao9/fix-roofline-analysis
Closed

Improve PyTorch profiler flop computation formulas#51377
xuzhao9 wants to merge 1 commit intopytorch:masterfrom
xuzhao9:xzhao9/fix-roofline-analysis

Conversation

@xuzhao9
Copy link
Copy Markdown
Contributor

@xuzhao9 xuzhao9 commented Jan 29, 2021

Summary:

Improve the flops computation formula of aten::conv2d operator to support stride, pad, dilation, and groups arguments.

This diff also fixes the following issues:

  • Apply a factor of 2 to aten::mm because output accounts for multiplication and addition.
  • Fix incorrect names of scalar operators to aten::mul and aten::add.

Test Plan:

python test/test_profiler.py

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Jan 29, 2021

💊 CI failures summary and remediations

As of commit b9e9205 (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 30, 2021

Codecov Report

Merging #51377 (b9e9205) into master (dbfaf96) will increase coverage by 0.00%.
The diff coverage is 89.28%.

@@           Coverage Diff           @@
##           master   #51377   +/-   ##
=======================================
  Coverage   80.86%   80.86%           
=======================================
  Files        1938     1938           
  Lines      211184   211221   +37     
=======================================
+ Hits       170767   170797   +30     
- Misses      40417    40424    +7     

Copy link
Copy Markdown
Contributor

@jspark1105 jspark1105 left a comment

Choose a reason for hiding this comment

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

Thanks for incorporating changes! Accepting but it would be great if @ilia-cher can also take a look.

Comment thread torch/csrc/autograd/profiler_utils.cpp Outdated
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.

PyTorch doesn't support having a different padding for left and right (or top and bottom)? Sorry I'm more familiar with Caffe2 conv operator.

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.

According to torch.nn.conv2d doc, padding only supports number of points in two directions and is applied on both sides: https://pytorch.org/docs/stable/generated/torch.nn.Conv2d.html

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@jspark1105 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@xuzhao9 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@xuzhao9 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary:

Improve the flops computation formula of aten::conv2d operator to support stride, pad, dilation, and groups arguments.

This diff also fixes the following issues:
- Apply a factor of 2 to aten::mm because output accounts for multiplication and addition.
- Fix incorrect names of scalar operators to aten::mul and aten::add.

Test Plan:

```python
python test/test_profiler.py
```

Reviewers:

Subscribers:

Tasks:

Tags:
@xuzhao9 xuzhao9 force-pushed the xzhao9/fix-roofline-analysis branch from bbf8537 to b9e9205 Compare February 1, 2021 21:32
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@xuzhao9 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@xuzhao9 merged this pull request in 4fdebdc.

@xuzhao9 xuzhao9 deleted the xzhao9/fix-roofline-analysis branch February 2, 2021 20:31
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Improve the flops computation formula of aten::conv2d operator to support stride, pad, dilation, and groups arguments.

This diff also fixes the following issues:
- Apply a factor of 2 to aten::mm because output accounts for multiplication and addition.
- Fix incorrect names of scalar operators to aten::mul and aten::add.

Pull Request resolved: pytorch#51377

Test Plan:
```python
python test/test_profiler.py
```

Reviewed By: jspark1105

Differential Revision: D26165223

Pulled By: xuzhao9

fbshipit-source-id: 2c5f0155c47af2e6a19332fd6ed73ace47fa072a
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.

4 participants