Dl/ov/tiny gpt2 example callbacks #20
Conversation
TensorReducerSequence Reducer adapter inside reducer TensorCollectorAdapter
github-actions
bot
added
experimental
NNCF Common
NNCF ONNX
NNCF OpenVINO
NNCF PT
NNCF TF
labels
There was a problem hiding this comment.
@AlexKoff88 - main file
| @staticmethod | ||
| def _get_callback(model, sequence_container): | ||
| original_model_outputs_names = {op.node.friendly_name for op in model.outputs} | ||
|
|
||
| def complition_callback(outputs): | ||
| for op, value in outputs.items(): | ||
| if op.node.friendly_name in original_model_outputs_names: | ||
| continue | ||
| if not isinstance(value, np.ndarray): | ||
| value = value.data | ||
| sequence_container[op.node.friendly_name].append(OVNNCFTensor(value)) | ||
|
|
||
| return complition_callback |
There was a problem hiding this comment.
@AlexKoff88 - callback creation for OpenVino
There was a problem hiding this comment.
@AlexKoff88 - custom inference in use here
There was a problem hiding this comment.
@AlexKoff88 - this example is working too
nncf/common/factory.py
Outdated
| @@ -35,7 +35,7 @@ def create(model: TModel) -> NNCFGraph: | |||
| from nncf.onnx.graph.nncf_graph_builder import GraphConverter | |||
|
|
|||
| return GraphConverter.create_nncf_graph(model) | |||
| if model_backend == BackendType.OPENVINO: | |||
| if model_backend in [BackendType.OPENVINO, BackendType.OPTIMUM]: | |||
There was a problem hiding this comment.
I didn't get why you need BackendType.OPTIMUM?
There was a problem hiding this comment.
This is redundant code from my previous experiments, please ignore
| if self._is_custom_inference: | ||
| sequence_container = defaultdict(list) | ||
| custom_forward = self.dataset.get_custom_forward( | ||
| engine.compiled_model, self._get_callback(model, sequence_container) |
There was a problem hiding this comment.
engine.compiled_model is supposed to be in the backend specific part
|
|
||
| def set_ov_model_in_hf_model(hf_model, ov_model): | ||
| hf_model.model = ov_model | ||
| hf_model.request = ov_model.create_infer_request() |
There was a problem hiding this comment.
I assume that ov_model has a type of ov::Model. If so, .create_infer_request() works only for CompiledModel
| set_ov_model_in_hf_model(hf_model, ov_model) | ||
|
|
||
| def _callback_fn(info): | ||
| outputs = {k: v for k, v in zip(info["infer_request"].model_outputs, info["infer_request"].outputs)} |
There was a problem hiding this comment.
Does the InferRequest object have .model_outputs property?
There was a problem hiding this comment.
Yes, and this attribute is used in HF integration https://github.com/huggingface/optimum-intel/blob/main/optimum/intel/openvino/modeling_decoder.py#L284-L287
| return data_item | ||
|
|
||
|
|
||
| dataset = nncf.CustomInferenceDataset([tokens] * 10, transform_fn, get_custom_forward) |
There was a problem hiding this comment.
I don't think we should make get_custom_forward a part of Dataset API. I propose:
- rename it to
get_forward_fn(model: ov.Model, output_processing_callback: Callable) -> Callable - make it an optional argument of nncf.quantize() API
There was a problem hiding this comment.
I absolutely agree that it should not be part of Dataset API.
Comments from my side:
I have some concerns about get_forward_fn:
output_processing_callbackis not needed for Torch and Keras TF models. It can confuse developer because they will calloutput_processing_callbackthat does not do it anything.- signature of
output_processing_callbackis not clear for different frameworks.
Proposal:
- Introduce
get_forward_fn(model: ov.Model) -> CallableTorch and Keras TF andget_forward_fn(model: ov.Model, statistic_aggregator: StatisticsAggregator) -> Callablefor OpenVINO, ONNX and TF.
Pros:
-
It is addressed to 1 via explicit introduction different signatures for different frameworks because different frameworks collect statistics with using different approaches.
-
It is addressed to 2 because methods of a class can be easily documented + sugar from IDE. It also can provide several interfaces to register model output.
statistic_collector.register_model_output(name, tensor) ``statistic_collector.register_model_outputs(outputs: Dict[str, tensor])
Request changes:
def get_custom_forward(ov_model, statistic_aggregator):
hf_model = model_with_pkv
set_ov_model_in_hf_model(hf_model, ov_model)
def _callback_fn(info):
outputs = {k.key.get_any_name(): v.value for k, v in zip(info["infer_request"].model_outputs, info["infer_request"].outputs)}
statistic_aggregator.register_model_outputs(outputs)
- Introduce a different classes to join framework model and custom forward function for each framework. For example
nncf.OVModelWithCustomForward(model: ov.Model, get_forward_fn: Callable)for OV
Pros:
nncf.quantizeandnncf.quantize_with_accuracy_controlw/o extending signature- The class explicitly specified signature of get_forward_fn for framework model.
- Easy reuse in other algorithms
ov_model_with_custom_forward = nncf.OVModelWithCustomForward(model_with_pkv.model, get_forward_fn)
quantized_model_with_custom_forward = nncf.quantize(ov_model_with_custom_forward, dataset, subset_size=3)
- IMHO: rename
get_forward_fn->make_forward_fn
There was a problem hiding this comment.
To tell the truth, I am still skeptical about the whole approach of collecting recurrent states and how this is applicable to other models. Now, I am looking at the Whisper notebook and I would not use this API since it requires much more effort and code rewriting to use the proposed API.
Changes