[TFLite] Update MultiplyByQuantizedMultiplier to use single-rounding instead of double-rounding #50290
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…f double-rounding
… outputs The change provides a more in-depth testing of the SUB operator. Previously the same scaling was used for both inputs and for the output, a simple subtraction in the kernel without any rescaling would have made the tests pass.
renjie-liu
reviewed
Jun 17, 2021
renjie-liu
approved these changes
Jul 3, 2021
google-ml-butler
bot
added
kokoro:force-run
|
Thank you for the review. @renjie-liu I split the PR in two. The #50615 PR changes the SUB and DIV kernels to avoid the usage of negative quantized multipliers in |
|
@Tessil Can you please address Ubuntu Sanity errors? Thanks! |
|
@gbaned Thanks, the latest commit should fix it. |
google-ml-butler
bot
added
kokoro:force-run
gbaned
approved these changes
Dec 2, 2021
google-ml-butler
bot
added
kokoro:force-run
gbaned
added
ready to pull
mansnils
added a commit
to mansnils/CMSIS_5
that referenced
this pull request
Dec 14, 2021
Adapt to Tflite/TfliteMicro, which will use single rounding instead of double rounding: tensorflow/tensorflow#50290 Changes are added under flag CMSIS_NN_USE_SINGLE_ROUNDING, which is disabled by default.
mansnils
added a commit
to ARM-software/CMSIS_5
that referenced
this pull request
Jan 3, 2022
Adapt to Tflite/TfliteMicro, which will use single rounding instead of double rounding: tensorflow/tensorflow#50290 Changes are added under flag CMSIS_NN_USE_SINGLE_ROUNDING, which is disabled by default.
felix-johnny
pushed a commit
to ARM-software/CMSIS-NN
that referenced
this pull request
Oct 4, 2022
Adapt to Tflite/TfliteMicro, which will use single rounding instead of double rounding: tensorflow/tensorflow#50290 Changes are added under flag CMSIS_NN_USE_SINGLE_ROUNDING, which is disabled by default.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
This PR changes the rounding of the int32
MultiplyByQuantizedMultipliermethods to a single-rounding instead of a double-rounding in a way similar to what was done in google/ruy#227 and adapts the tests to the change.The new int32
MultiplyByQuantizedMultiplieradds a restriction on the quantized multiplier, it must now be a positive integer in the same way as in the int64 version ofMultiplyByQuantizedMultiplier. This restriction also matches the one in the quantization scaling defined in the TOSA specification. To achieve that the SUB and DIV operators were slightly adapted to avoid any negative quantized multiplier.Thibaut