Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Good First Issue][NNCF]: Dump actual_subset_size to ov.Model #2562

Closed
l-bat opened this issue Mar 8, 2024 · 16 comments · Fixed by #2995
Closed

[Good First Issue][NNCF]: Dump actual_subset_size to ov.Model #2562

l-bat opened this issue Mar 8, 2024 · 16 comments · Fixed by #2995
Assignees
Labels
good first issue Good for newcomers

Comments

@l-bat
Copy link
Collaborator

l-bat commented Mar 8, 2024

Context

After applying quantization to the ov.Model in Neural Network Compression Framework (NNCF), the quantization parameters, including subset_size, are dumped to the meta section of the OpenVINO IR. subset_size represents the size of the dataset used for calibration.

"subset_size": subset_size,

However, inconsistencies arise when the dataset size is less than the provided or default 'subset_size'. To address this confusion, it is proposed to also dump the actual_subset_size, which denotes the number of data samples used to calculate activation statistics. This addition will improve clarity and accuracy in managing quantization parameters and assist in reproducing quantization results.

What needs to be done?

  • Dump actual_subset_size parameter to ov.Model meta section.
  • Add tests

Example Pull Requests

No response

Resources

Contact points

@l-bat

Ticket

No response

@l-bat l-bat added the good first issue Good for newcomers label Mar 8, 2024
@github-project-automation github-project-automation bot moved this to Contributors Needed in Good first issues Mar 8, 2024
@AiGaf1
Copy link

AiGaf1 commented Mar 9, 2024

.take

Copy link

github-actions bot commented Mar 9, 2024

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

@alexsu52 alexsu52 moved this from Contributors Needed to Assigned in Good first issues Mar 11, 2024
@andrey-churkin
Copy link
Contributor

@AiGaf1 Hi, are you still working on this task? Do you need any help? Please inform us if you do not plan to continue working on this task. Thanks!

AiGaf1 added a commit to AiGaf1/nncf that referenced this issue Mar 22, 2024
Dump actual_subset_size to ov.Model (Issue openvinotoolkit#2562)
@RitikaxShakya
Copy link

RitikaxShakya commented Mar 29, 2024

Hello! is there any update on this issue? If not i wish to work on this issue.

@p-wysocki
Copy link
Contributor

p-wysocki commented Apr 3, 2024

@l-bat could you please reassign the issue to @RitikaxShakya? I lack the permissions for NNCF repository.

@RitikaxShakya
Copy link

.take

Copy link

github-actions bot commented Apr 4, 2024

Thanks for being interested in this issue. It looks like this ticket is already assigned to a contributor. Please communicate with the assigned contributor to confirm the status of the issue.

@alexsu52 alexsu52 assigned RitikaxShakya and unassigned AiGaf1 Apr 18, 2024
@p-wysocki
Copy link
Contributor

Hello @RitikaxShakya, are you still working on that issue? Do you need any help?

@RitikaxShakya RitikaxShakya removed their assignment Jun 20, 2024
@alexsu52 alexsu52 moved this from Assigned to Contributors Needed in Good first issues Jun 27, 2024
@awayzjj
Copy link

awayzjj commented Jun 27, 2024

.take

Copy link

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

@alexsu52 alexsu52 moved this from Contributors Needed to Assigned in Good first issues Jun 28, 2024
@awayzjj
Copy link

awayzjj commented Jun 29, 2024

Hi @l-bat I created a PR after testing locally, and the XML output is as expected:
截屏2024-06-29 11 46 58

I ran the pytest in tests/openvino, and the original tests did not break. But I have 2 questions

  1. which file I should edit to add my unit test.
  2. how to implement the unit test, should I check the output XML to verify whether the actual_subset_size property exists?

Thank you very much!

@alexsu52 alexsu52 moved this from Assigned to In Review in Good first issues Jul 1, 2024
@l-bat
Copy link
Collaborator Author

l-bat commented Jul 1, 2024

Hi @awayzjj!
Thanks for your contribution!

calibration_dataset.get_length() returns the size of the dataset that was provided to the nncf.quantize method, however actual_subset_size should show the number of data samples that were used to calculate the activation statistics.
In the case of calibration_dataset.get_length() >= subset_size, actual_subset_size is equal to subset_size. Otherwise, actual_subset_size must be equal to calibration_dataset.get_length(). But it is not possible to use the get_length() method if __len__() is not implemented. Please take a look at

dataset_length = self.dataset.get_length()
if dataset_length and self.stat_subset_size:
return min(dataset_length, self.stat_subset_size)
return dataset_length or self.stat_subset_size
. You can implement the get_actual_subset_size() function.

@l-bat
Copy link
Collaborator Author

l-bat commented Jul 1, 2024

I ran the pytest in tests/openvino, and the original tests did not break. But I have 2 questions

  1. which file I should edit to add my unit test.
  2. how to implement the unit test, should I check the output XML to verify whether the actual_subset_size property exists?
  1. You can add test to https://github.com/openvinotoolkit/nncf/blob/e8ea2521663de807d654ae4f375d20c904755061/tests/openvino/native/quantization/test_quantization_pipeline.py
  2. You can use the test as an example
    def test_ignored_scope_dump(ignored_options, expected_dump, tmp_path):
    ignored_scope_path = ["nncf", "quantization", "ignored_scope"]
    model = WeightsModel().ov_model
    dataset = get_dataset_for_test(model)
    quantize_parameters = {
    "preset": QuantizationPreset.PERFORMANCE,
    "target_device": TargetDevice.CPU,
    "subset_size": 1,
    "fast_bias_correction": True,
    "ignored_scope": ignored_options,
    }
    quantized_model = quantize_impl(model, dataset, **quantize_parameters)
    ov.save_model(quantized_model, tmp_path / "ov_model.xml")
    core = ov.Core()
    dumped_model = core.read_model(tmp_path / "ov_model.xml")
    for key, value in expected_dump.items():
    rt_path = ignored_scope_path + [key] if key else ignored_scope_path
    if value:
    assert dumped_model.get_rt_info(rt_path) == value
    else:
    assert dumped_model.has_rt_info(rt_path) is False

@awayzjj
Copy link

awayzjj commented Aug 26, 2024

@l-bat Hi, I've been really busy lately, so I've decided to unassign myself for now. I apologize for any inconvenience this may cause.

@awayzjj awayzjj removed their assignment Aug 26, 2024
@alexsu52 alexsu52 moved this from In Review to Contributors Needed in Good first issues Aug 29, 2024
@zina-cs
Copy link
Contributor

zina-cs commented Sep 26, 2024

.take

Copy link

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

@alexsu52 alexsu52 moved this from Contributors Needed to Assigned in Good first issues Sep 26, 2024
@github-project-automation github-project-automation bot moved this from Assigned to Closed in Good first issues Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Archived in project
7 participants