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

Dynamic OV model builder #3137

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

KodiaqQ
Copy link
Collaborator

@KodiaqQ KodiaqQ commented Dec 11, 2024

Changes

  • Added ModelBuilder class.
  • Updated FastBC algorithm to utilize new approach.

Reason for changes

  • Algorithm speed up.

Related tickets

  • 122317

Tests

  • TBD

@github-actions github-actions bot added NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PTQ Pull requests that updates NNCF PTQ labels Dec 11, 2024
@KodiaqQ KodiaqQ closed this Dec 11, 2024
@KodiaqQ KodiaqQ reopened this Dec 12, 2024
@github-actions github-actions bot added the NNCF PT Pull requests that updates NNCF PyTorch label Dec 12, 2024
@github-actions github-actions bot removed the NNCF PT Pull requests that updates NNCF PyTorch label Dec 18, 2024
# We have to have args as the inputs since all of them are nodes and are required to be as input.
args = [clone_nodes.popleft() for _ in graph_node.inputs()]

clone_node = self._node_factory.create(node_type, args, attrs)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, NodeFactory will create nodes without names since naming is done using a decorator (see https://github.com/openvinotoolkit/openvino/blob/master/src/bindings/python/src/openvino/utils/decorators.py#L25-L34).
It shouldn't be a problem, since the FastBC algorithm utilizes Parameter and Result nodes for statistics. But for BiasCorrection algorithm utilizes the names.
The current approach may be changed by adding the corresponding constructor calls into metatypes, for example, for each layer.

Comment on lines +136 to +137
if node_mapping is None:
node_mapping = {op.get_friendly_name(): op for op in model.get_ops()}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This needs to avoid failures for the algorithms that utilize get_bias_value, but without mapping for now.

Comment on lines +123 to +129
def extract_submodel(self, model_transformer, input_id, output_id):

return self._model_builder.build(
input_ids=[input_id],
output_ids=[output_id],
node_mapping=self._node_mapping,
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To keep the original behaviour of the other backends and reduce code changing, the overloading approach was used here.

@KodiaqQ KodiaqQ changed the title FBC test Dynamic OV model builder Dec 18, 2024
@KodiaqQ
Copy link
Collaborator Author

KodiaqQ commented Dec 18, 2024

@alexsu52, @andrey-churkin, could you please review not PR (need to discuss a number of questions), but the proposal of the new graph-building approach (for FastBiasCorrection and as the possible extension of the BiasCorrection)? Thanks.
All details were stored in the corresponding ticket. See the latest comments.
cc @MaximProshin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PTQ Pull requests that updates NNCF PTQ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant