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.

Sign up for GitHub

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

Jump to bottom

dump actual_subset_size #2769

Closed
wants to merge 2 commits into from
Closed

dump actual_subset_size #2769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift click to select a range
964c349
dump actual_subset_size
awayzjj Jun 29, 2024
05f9bcf
implement get_actual_subset_size and add test
awayzjj Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions nncf/openvino/quantization/quantize_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 57,13 @@
TTensor = TypeVar("TTensor")


def get_actual_subset_size(dataset, subset_size):
dataset_length = dataset.get_length()
if dataset_length:
return min(dataset_length, subset_size)
return subset_size
Copy link
Collaborator

Choose a reason for hiding this comment

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

if __len()__ is not implemented, the actual subset size should be obtained as:

actual_subset_size = 0
for data in islice(calibration_dataset.get_inference_data(), subset_size):
    actual_subset_size  = 1

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the late reply. I found that the suggested changes do not work because the calibration_dataset.get_inference_data() iterator is consumed before dump_parameters. As a result, actual_subset_size ends up being zero. Could you please provide some suggestions on how to handle this issue?
Thank you very much! @l-bat

Copy link
Collaborator

Choose a reason for hiding this comment

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

@awayzjj after discussing within the team, we decided not to add actual_subset_size to rt_info because rt_info is intended to contain quantization parameters provided to algorithms. If the provided dataset is shorter than subset_size, we should display a warning during the statistics collection process:

nncf/nncf/common/tensor_statistics/aggregator.py

Line 76 in 408c67d

description="Statistics collection",



@tracked_function(NNCF_OV_CATEGORY, [CompressionStartedWithQuantizeApi(), "target_device", "preset"])
def native_quantize_if_op_impl(
model: ov.Model,
Expand Down Expand Up @@ -131,6 138,7 @@ def _extract_all_subgraphs(model: ov.Model, current_id: str) -> None:
"preset": preset,
"target_device": target_device.value,
"subset_size": subset_size,
"actual_subset_size": get_actual_subset_size(calibration_dataset, subset_size),
"fast_bias_correction": fast_bias_correction,
"model_type": model_type,
"ignored_scope": ignored_scope,
Expand Down Expand Up @@ -179,6 187,7 @@ def native_quantize_impl(
"preset": preset,
"target_device": target_device.value,
"subset_size": subset_size,
"actual_subset_size": get_actual_subset_size(calibration_dataset, subset_size),
"fast_bias_correction": fast_bias_correction,
"model_type": model_type,
"ignored_scope": ignored_scope,
Expand Down Expand Up @@ -318,6 327,7 @@ def native_quantize_with_accuracy_control_impl(
"preset": preset,
"target_device": target_device.value,
"subset_size": subset_size,
"actual_subset_size": get_actual_subset_size(calibration_dataset, subset_size),
"fast_bias_correction": fast_bias_correction,
"model_type": model_type,
"ignored_scope": ignored_scope,
Expand Down
17 changes: 17 additions & 0 deletions tests/openvino/native/quantization/test_quantization_pipeline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 197,20 @@ def test_ignored_scope_dump(ignored_options, expected_dump, tmp_path):
assert dumped_model.get_rt_info(rt_path) == value
else:
assert dumped_model.has_rt_info(rt_path) is False


@pytest.mark.parametrize("subset_size, expected_actual_subset_size", [[1, 1], [2, 1]])
def test_dump(subset_size, expected_actual_subset_size, tmp_path):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add 2 tests:

  1. __len()__ implemented
  2. __len()__ not implemented

model = WeightsModel().ov_model
dataset = get_dataset_for_test(model) # dataset.get_length() == 1
quantize_parameters = {
"preset": QuantizationPreset.PERFORMANCE,
"target_device": TargetDevice.CPU,
"subset_size": subset_size,
"fast_bias_correction": True,
}
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")
assert dumped_model.get_rt_info(["nncf", "quantization", "actual_subset_size"]) == str(expected_actual_subset_size)
Loading