openvinotoolkit / nncf

Neural Network Compression Framework for enhanced OpenVINO™ inference
Apache License 2.0
945 stars 235 forks source link

[Good First Issue][NNCF]: Replace common tensor_statistics by experimental tensor_statistics #3041

Open kshpv opened 3 weeks ago

kshpv commented 3 weeks ago

Context

Historically, NNCF has maintained two sets of tensor statistics: the old tensor statistics and the new tensor statistics. The new tensor statistics, located in the experimental/common/tensor_statistics directory, offer improved functionality and better performance. However, the codebase still contains references to the old tensor statistics, leading to redundancy and potential confusion.

What needs to be done?

  1. Search the codebase for all instances where the old tensor statistics are being used. This includes algorithms, tests, and any utility functions.
  2. Modify the identified algorithms to utilize the new tensor statistics from experimental/common/tensor_statistics.
  3. Update the corresponding tests to ensure they are compatible with the new tensor statistics.
  4. Replace the contents of common/tensor_statistics/collectors with the corresponding implementations from experimental/common/tensor_statistics.
  5. Once all references to the old tensor statistics have been updated, remove the experimental/common/tensor_statistics.
  6. Remove common/tensor_statistics/reduction if it is not needed anymore.

In the end, there should be no experimental/common/tensor_statistics.

Example Pull Requests

https://github.com/openvinotoolkit/nncf/pull/2117

Resources

Contact points

@kshpv

olegkkruglov commented 3 weeks ago

.take

github-actions[bot] commented 3 weeks ago

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

AHB102 commented 3 weeks ago

@kshpv So basically functionally replace everything in old files(common) with the new files(Experimental) and then delete the new files(Experimental) ?

kshpv commented 3 weeks ago

@AHB102, you also need to be sure that all tests passed and no regression is introduced

AHB102 commented 3 weeks ago

@kshpv Great! I've reviewed the example and will create a PR for a single-file change. We can go from there

kshpv commented 3 weeks ago

@AHB102 I am really sorry, but @olegkkruglov already picked this GFI. I suggest you to pick another one. We have many things to do. I think we probably open new GFIs in the following days. If you are interested I can ping you. What do you say?

AHB102 commented 3 weeks ago

@kshpv Yup no problem 😄

mlukasze commented 2 weeks ago

hey @olegkkruglov - do you need any support or you may not have a time to finish the task?

olegkkruglov commented 2 weeks ago

hey, the work is in progress. sorry, I didn't have enough time to finish. I think I will be able to do it during this weekend, is it ok?

mlukasze commented 2 weeks ago

of course :)