Skip to content

Count the operator spelling of add and mul in the flops profiler - #8509

Open
ebarkhordar wants to merge 1 commit into
deepspeedai:masterfrom
ebarkhordar:fix/3087-flops-profiler-operator-dunders
Open

ebarkhordar wants to merge 1 commit into
deepspeedai:masterfrom
ebarkhordar:fix/3087-flops-profiler-operator-dunders

Conversation

@ebarkhordar

Copy link
Copy Markdown
Contributor

get_model_profile reports 0 flops for an elementwise op written as x + y, and one flop per element for the same op written as torch.add(x, y). The profiler patches torch.add, torch.Tensor.add, torch.mul and torch.Tensor.mul, but not the operator forms, so every residual connection in a profiled model is counted as free. On the reporter's script at c102bbd1, x + x over 20 elements gives 0 where torch.add(x, x) gives 20.

torch.Tensor.__matmul__ is already patched for this reason (#4724), so the list is incomplete rather than deliberately limited. This adds __add__, __radd__, __mul__ and __rmul__ beside it, with matching entries in _reload_tensor_methods so the names are restored when profiling ends.

The reflected forms are here so that 2.0 * x and x * 2.0 agree. In-place __iadd__ and __imul__ are left out on purpose: Tensor.add_ and Tensor.mul_ are not counted either, and adding one without the other recreates the same asymmetry a level down. x - y and x / y also report 0, but torch.sub and torch.div are missing from the patch list in every spelling, so that is a separate change and this one is Refs rather than Fixes.

How I checked it, on CPU with torch 2.8.0:

  • The new tests compare the three spellings of add and mul, and a scalar operand on either side. They fail on master (0 against 24) and pass here.
  • torch.add(x, x) and x.add(x) still report 20, so reaching the op through the operator does not count it twice.
  • LeNet5 in the existing inference test is unchanged at 866076672 flops and 426516480 macs. A residual block x + self.fc(x) moves from 200 to 210, which is the 10-element add.
  • pytest -m sequential unit/profiling/flops_profiler/ passes 23, and every pre-commit hook passes on both changed files, with flake8 run under Python 3.10 because its pinned version needs one.

There is no GPU on the machine I used, so I have not exercised the CUDA or precompile paths, and I tested only against torch 2.8.0.

Refs #3087

The profiler patches torch.add, torch.Tensor.add, torch.mul and torch.Tensor.mul,
but not the operator forms, so an elementwise op written as x + y reported zero
flops while the same op written as torch.add(x, y) reported one flop per element.
Every residual connection in a profiled model was invisible.

torch.Tensor.__matmul__ is already patched for this reason, so this adds the same
treatment for __add__, __radd__, __mul__ and __rmul__, and the matching entries in
_reload_tensor_methods so the names are restored when profiling ends.

Refs deepspeedai#3087

Signed-off-by: Ehsan Barkhordar <realbarkhordar@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df5f2284a7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


torch.mul = wrapFunc(torch.mul, _mul_flops_compute)
torch.Tensor.mul = wrapFunc(torch.Tensor.mul, _mul_flops_compute)
torch.Tensor.__mul__ = wrapFunc(torch.Tensor.__mul__, _mul_flops_compute)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add the mandatory sign-off trailer

This is a non-merge commit with one parent, but its message contains no Signed-off-by trailer. The root AGENTS.md requires every non-merge commit to be signed off, so this commit does not meet the repository's commit/CI requirements until the author trailer is added.

AGENTS.md reference: AGENTS.md:L6-L10

Useful? React with 👍 / 👎.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant