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

Add model compression to FP16 weights #7588

Merged
merged 18 commits into from
Nov 9, 2021

Conversation

mvafin
Copy link
Contributor

@mvafin mvafin commented Sep 21, 2021

Details:

  • Add new command line argument to MO to make compression
  • Make a note in the old data_type option that it is deprecated

Tickets:

  • 64953

@mvafin mvafin requested review from a team September 21, 2021 20:56
@openvino-pushbot openvino-pushbot added category: Python API OpenVINO Python bindings category: MO Model Optimizer labels Sep 21, 2021
@mvafin mvafin requested a review from a team September 27, 2021 09:38
@mvafin mvafin requested review from a team as code owners September 29, 2021 19:46
@mvafin mvafin requested a review from a team October 7, 2021 11:26
@mvafin mvafin force-pushed the api2/compression branch 2 times, most recently from 722c6c7 to 6f254d2 Compare October 7, 2021 11:58
@mvafin
Copy link
Contributor Author

mvafin commented Oct 7, 2021

Had to force rebase, because of changes in third-party which I couldn't revert otherwise

@mvafin mvafin requested a review from a team October 20, 2021 21:57
@mvafin mvafin force-pushed the api2/compression branch 2 times, most recently from 246618b to 1789616 Compare October 21, 2021 13:10
namespace ov {
namespace pass {

class TRANSFORMATIONS_API CompressFloatConstantsImpl;
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to reveal Impl in API?

* @ingroup ie_transformation_common_api
* @brief CompressFloatConstants transformation replaces FP32/FP64 Constants with FP16 ones.
*/
class ov::pass::CompressFloatConstants : public ov::pass::GraphRewrite {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems not only about constant compression. It also covers parameters. I propose to rename it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It changes constants, parameters only get old api map with runtime info, CompressFloatConstants describe best I think.

order.resize(r);
std::iota(order.begin(), order.end(), 0);
} else {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

why can't we handle dynamic rank and use {} for order?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

old api map is used for old api, right? Old api doesnt support dynamic rank anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

This is open question. It makes sense to discuss offline how we handle dynamic rank paramters.

// Copyright (C) 2018-2021 Intel Corporation
// SPDX-License-Identifier: Apache-2.0
//

Copy link
Contributor

Choose a reason for hiding this comment

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

propose to combine these routines with decompression* routines in one place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not a good idea in my opinion, all attribute are located in individual files, which is easier to manage

@evgenytalanin-intel evgenytalanin-intel merged commit 34886b6 into openvinotoolkit:master Nov 9, 2021
OpenVINO-dev-contest pushed a commit to OpenVINO-dev-contest/openvino that referenced this pull request Nov 23, 2021
* Add model compression to FP16 weights

* Fix build

* Fix build

* Fix build

* Add wrapper over ConvertPrecision

* Add documentation to attributes

* Fix MO IR Reader

* Fix build

* Return DisableDecompressionConvertConstantFolding call in CommonOptimizations

* Temporarily disable old_api map

* Fix TI Convert issue

* Apply review feedback

* Fix build

* Fix build

* Fix build
OpenVINO-dev-contest pushed a commit to OpenVINO-dev-contest/openvino that referenced this pull request Nov 24, 2021
* Add model compression to FP16 weights

* Fix build

* Fix build

* Fix build

* Add wrapper over ConvertPrecision

* Add documentation to attributes

* Fix MO IR Reader

* Fix build

* Return DisableDecompressionConvertConstantFolding call in CommonOptimizations

* Temporarily disable old_api map

* Fix TI Convert issue

* Apply review feedback

* Fix build

* Fix build

* Fix build
@mvafin mvafin deleted the api2/compression branch August 10, 2022 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: MO Model Optimizer category: Python API OpenVINO Python bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants