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

Concat Threshold Update #1016

Closed
wants to merge 33 commits into from
Closed

Conversation

samuel-wj-chapman
Copy link
Collaborator

@samuel-wj-chapman samuel-wj-chapman commented Mar 25, 2024

Pull Request Description:

feature to force the alignment of thresholds pre and post concatenation layer. implemented for both torch and keras. Including testing for each.

update does not occur when previous layer has multiple outputs.

Checklist before requesting a review:

  • I set the appropriate labels on the pull request.
  • I have added/updated the release note draft (if necessary).
  • I have updated the documentation to reflect my changes (if necessary).
  • All function and files are well documented.
  • All function and classes have type hints.
  • There is a licenses in all file.
  • The function and variable names are informative.
  • I have checked for code duplications.
  • I have added new unittest (if necessary).

@@ -0,0 +1,59 @@
# Copyright 2022 Sony Semiconductor Israel, Inc. All rights reserved.
#
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please fix the license year.


MATCHER = NodeOperationMatcher(Concatenate)

class threshold_updater(common.BaseSubstitution):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please rename the class to include 'concat' or something like that (for example: ConcatThresholdUpdate)

from model_compression_toolkit.core.common.graph.graph_matchers import NodeOperationMatcher
from model_compression_toolkit.constants import THRESHOLD

MATCHER = NodeOperationMatcher(Concatenate)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about tf.concat?

for prev_node in prev_nodes:
if len(graph.get_next_nodes(prev_node))==1:
prev_node.candidates_quantization_cfg[0].activation_quantization_cfg.activation_quantization_params[THRESHOLD] = concat_threshold

Copy link
Collaborator

Choose a reason for hiding this comment

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

We also need to deal with mixed-precision case since the threshold can be different for each cfg in candidates_quantization_cfg.

Copy link
Collaborator

Choose a reason for hiding this comment

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

several important things:

  1. we need to check that both the concat node and the previous node have a 'threshold' - this will ensure only symmetric or power of 2 quantization (and not uniform)
  2. we need to check that we have only a single candidate cfg - since we want to avoid the substitution in case of mixed precision.
  3. maybe we can add a "todo" comment that it can be expanded to mixed precision and to uniform quantizer in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed code to leave the case of MP. will add a fix if results for the method are good for ptq.

@samuel-wj-chapman
Copy link
Collaborator Author

samuel-wj-chapman commented Mar 29, 2024

made the relevant changes. let me know if i have filled in all the correct documentation.

for prev_node in prev_nodes:
if len(graph.get_next_nodes(prev_node))==1:
prev_node.candidates_quantization_cfg[0].activation_quantization_cfg.activation_quantization_params[THRESHOLD] = concat_threshold

Copy link
Collaborator

Choose a reason for hiding this comment

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

several important things:

  1. we need to check that both the concat node and the previous node have a 'threshold' - this will ensure only symmetric or power of 2 quantization (and not uniform)
  2. we need to check that we have only a single candidate cfg - since we want to avoid the substitution in case of mixed precision.
  3. maybe we can add a "todo" comment that it can be expanded to mixed precision and to uniform quantizer in the future.

model_compression_toolkit/core/quantization_prep_runner.py Outdated Show resolved Hide resolved
samuel-wj-chapman and others added 12 commits April 4, 2024 11:09
addition of zscore mct tutorial including updates to tutorial utils
…#1022)

Co-authored-by: Ofir Gordon <Ofir.Gordon@altair-semi.com>
* Update docs and version number
---------

Co-authored-by: Ofir Gordon <Ofir.Gordon@altair-semi.com>
Use TF 2.15 for test_suit_pip_keras.yml
* Yolov8n tutorial - add gptq option for optimized  for imx500
…ny#1000)

* Update nanodet-keras-model in mct_model_garden to support configurable number of classes
lapid92 and others added 9 commits April 8, 2024 14:12
…izers fro all modules in model (instaed of childrens) (sony#1028)
* Increase coverage threshold for test success to 96%
* Include only model_compression_toolkit files in the report.
* Archive an HTML coverage report.
* Remove unused files like model_compression_toolkit/core/exporter.py and model_compression_toolkit/core/keras/graph_substitutions/substitutions/remove_relu_upper_bound.py.

---------

Co-authored-by: reuvenp <reuvenp@altair-semi.com>
* Yolov8 Object Detection PyTorch tutorial
* Add Yolov8 Object Detection to the PyTorch MCT Model Garden
* The  PyTorch implementation of Yolov8n object detection model, following https://github.com/ultralytics/ultralytics. This implementation includes a slightly modified version of the yolov8 detection-head optimized for model quantization.
This commit fixes these issues: 
* The float graph used for comparison in the similarity analyzer underwent stats correction.
* The number of samples to display did not work as expected, and a single sample was always used.
* An error was thrown when no comparison points were found.

The fixes are done by:
* Changing the analyzer_model_quantization API to receive both float and quantized graphs.
* Add a method to the visualizer to query the existence of compare points and skip the analyzer with a warning in case no points were found.
* Change the computation of the number of visualized samples.

---------

Co-authored-by: reuvenp <reuvenp@altair-semi.com>
@samuel-wj-chapman
Copy link
Collaborator Author

closing pr due to commit issues re opoened as pr 1036

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants