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

Specification for UniformQuantizeOp and UniformDequantizeOp #1496

Merged
merged 7 commits into from
Jun 19, 2023

Conversation

sdasgup3
Copy link
Member

@sdasgup3 sdasgup3 commented May 18, 2023

fixes #531
fixes #530

Summary

The PR proposes the specification for uniform.quantize and uniform.dequantize ops.

The specification of uniform.quantize also captures the re-quantization conversions from quantized tensor to quantized tensors.

Please let me know your feedback.

Working notes on following the reference_checklist.md and spec_checkilist.md

uniform_dequantize

We have the following constraints from the spec

(I1)  `operand` is a  quantized tensor
(C1) `shape(operand) = shape(result)`.
(C2) `element_type(result) = expressed_type(operand)`.

These constraints will be comprehensively covered by the following tests:

I1: a) `operand: quantized tensors`. (Covered by ODS).
C1: a) `shape(operand) != shape(result)`. (Covered by ODS)
C2: a) `element_type(result) != expressed_type(operand)`.

If we drop the "Covered by ODS" pieces, this will leave us with the following test cases:

C2: a) `element_type(result) != expressed_type(operand)`.

We already has a type inference test to cover the above.

uniform_quantize

We have the following constraints from the spec

(I1)  `operand: tensor of floating-point or quantized type`.
(C1) `shape(operand) = shape(result)`.
(C2) `expressed_type(result) = is_float(operand) ? element_type(operand) :
  expressed_type(operand)`.

These constraints will be comprehensively covered by the following tests:

I1: a) `operand: quantized tensors`. (Covered by ODS).
C1: a) `shape(operand) != shape(result)`. (Covered by ODS)
C2: a) if_float(operand): `expressed_type(result) != element_type(operand)`.
      b) if_quantized(operand): `expressed_type(result) !=  expressed_type(operand)`.

If we drop the "Covered by ODS" pieces, this will leave us with the following test cases:

C2: a) if_float(operand): `expressed_type(result) != element_type(operand)`.
      b) if_quantized(operand): `expressed_type(result) !=  expressed_type(operand)`.

The above will be covered as part of #1603.

@sdasgup3 sdasgup3 added the Spec label May 18, 2023
@sdasgup3 sdasgup3 requested a review from burmako May 18, 2023 00:40
@sdasgup3 sdasgup3 self-assigned this May 18, 2023
docs/spec.md Outdated Show resolved Hide resolved
Copy link

@loganchien loganchien left a comment

Choose a reason for hiding this comment

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

Overall LGTM, which a few minor comments.

docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
@sdasgup3 sdasgup3 force-pushed the spec-uniform-quant-dequant branch 2 times, most recently from f8fcb10 to e0ef8e3 Compare May 23, 2023 17:14
Copy link

@dominicsymes dominicsymes left a comment

Choose a reason for hiding this comment

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

This looks fine to me. I think allowing also QuantizationIntegerScale should be discussed following that PR (#1475).

@sdasgup3
Copy link
Member Author

@dominicsymes
Thanks for the review.

I think allowing also QuantizationIntegerScale should be discussed following that PR

That is right. Once we have #1475 landed, then the quantize/dequantize op will allow that type too.

@burmako burmako assigned burmako and unassigned sdasgup3 Jun 1, 2023
docs/spec.md Show resolved Hide resolved
@sdasgup3 sdasgup3 force-pushed the spec-uniform-quant-dequant branch from e0ef8e3 to 340f968 Compare June 4, 2023 16:58
docs/spec.md Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
@burmako burmako assigned sdasgup3 and unassigned burmako Jun 4, 2023
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
@sdasgup3 sdasgup3 force-pushed the spec-uniform-quant-dequant branch from 340f968 to 03f1a16 Compare June 8, 2023 18:54
@sdasgup3 sdasgup3 force-pushed the spec-uniform-quant-dequant branch 2 times, most recently from eb3de74 to 52a5acf Compare June 8, 2023 22:23
@sdasgup3 sdasgup3 assigned burmako and unassigned sdasgup3 Jun 8, 2023
@sdasgup3
Copy link
Member Author

sdasgup3 commented Jun 8, 2023

My proposal is to merge the current PR and consider merging the two ops to convert op at a later point via #1576.

@sdasgup3 sdasgup3 requested review from dansuh17 and burmako June 8, 2023 22:28
docs/spec.md Show resolved Hide resolved
stablehlo/dialect/StablehloOps.td Outdated Show resolved Hide resolved
docs/spec.md Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
sdasgup3 added a commit that referenced this pull request Jun 19, 2023
As brought up
[here](#1496 (comment))
that some parts of the `reference_checklist.md` are relevant enough to
be included in the `spec_checklist.md`.
@sdasgup3 sdasgup3 assigned burmako and unassigned sdasgup3 Jun 19, 2023
@sdasgup3 sdasgup3 requested a review from burmako June 19, 2023 00:31
stablehlo/dialect/StablehloOps.td Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
stablehlo/tests/ops_stablehlo.mlir Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
@burmako burmako assigned sdasgup3 and unassigned burmako Jun 19, 2023
@sdasgup3 sdasgup3 merged commit 8993ad5 into openxla:main Jun 19, 2023
sdasgup3 added a commit to sdasgup3/stablehlo that referenced this pull request Jun 20, 2023
sdasgup3 added a commit to sdasgup3/stablehlo that referenced this pull request Jun 20, 2023
@burmako burmako added the Migrate to MHLO PR that needs to be migrated to MLIR-HLO label Jun 25, 2023
copybara-service bot pushed a commit to tensorflow/mlir-hlo that referenced this pull request Jun 26, 2023
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Jun 26, 2023
copybara-service bot pushed a commit to openxla/xla that referenced this pull request Jun 26, 2023
sdasgup3 added a commit to sdasgup3/stablehlo that referenced this pull request Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Migrate to MHLO PR that needs to be migrated to MLIR-HLO Spec
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add spec for UniformQuantizeOp Add spec for UniformDequantizeOp
7 participants