search

sovrasov / flops-counter.pytorch

Flops counter for convolutional networks in pytorch framework
MIT License
2.82k stars 307 forks source link

The sum of per layer flops doesn't match the total flops #62

Closed yuzhms closed 3 years ago

yuzhms commented 3 years ago

If a module contain two types of operations:

The results output by ptflops will be misleading. Since in the calculation of full module's flops:

https://github.com/sovrasov/flops-counter.pytorch/blob/fc2c67d8d653fbe086af7e07dd9ca96aa97268d0/ptflops/flops_counter.py#L186

It just counts all flops of modules() output.

But to output per layer flops:

https://github.com/sovrasov/flops-counter.pytorch/blob/fc2c67d8d653fbe086af7e07dd9ca96aa97268d0/ptflops/flops_counter.py#L107

it ignores flops which a module's parent is in supported modules.

This mismatch cause the final results pretty confusing.

I think you should at least make these two types of calculation consistent.

sovrasov commented 3 years ago

Hi! Operations that are defined directly are not handled by ptflops. It handles supported modules only. Thus, start_flops_count doesn't take into account torch.matmul. So I think both outputs are consistent. Could you please provide a code to reproduce the issue?

yuzhms commented 3 years ago

Hi. let me make it clear at first. I am not fire an issue but request a feature.

I am aware the ptflops does not support directly defined operations. And we can write a module to wrap the directly defined operations and a custom hook to make it works for ptflops. Right?

However, when I use some existing models, and they use a hybridized way to define a custom model, which use both torch's module and directly defined operations. If I want to count flops of this model, I have to change the origional code: moving the directly defined operations into a module.

I have noticed that the accumulate_floops can handle nested module well, sicne it checks whether a module is supported before moving into its children. So why not use the same mechanism in the start_floops_count ?

sovrasov commented 3 years ago

This change is possible. There is a duplication: compute_average_flops_cost and print_model_with_flops do the same job and can be merged.