Specification of quantized reduce and other related ops
#1796
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
loganchien
reviewed
Oct 9, 2023
burmako
suggested changes
Oct 10, 2023
sdasgup3
force-pushed
the
reduce-impl
branch
3 times, most recently
from
2d287df to
3debca6
Compare
loganchien
approved these changes
Oct 10, 2023
sdasgup3
force-pushed
the
reduce-impl
branch
2 times, most recently
from
a326fb5 to
bfd96a2
Compare
loganchien
approved these changes
Oct 10, 2023
burmako
suggested changes
Oct 11, 2023
sdasgup3
changed the title
reduce, reduce_window and select_and_scatter opsreduce and other related ops
doyeonkim0
reviewed
Oct 13, 2023
doyeonkim0
reviewed
Oct 13, 2023
doyeonkim0
reviewed
Oct 13, 2023
doyeonkim0
reviewed
Oct 13, 2023
sdasgup3
force-pushed
the
reduce-impl
branch
4 times, most recently
from
2d4f432 to
ada3d64
Compare
doyeonkim0
approved these changes
Oct 16, 2023
dominicsymes
approved these changes
Oct 16, 2023
paulinesho
approved these changes
Nov 7, 2023
sdasgup3
force-pushed
the
reduce-impl
branch
from
ada3d64 to
4e6768b
Compare
paulinesho
approved these changes
Nov 15, 2023
sdasgup3
force-pushed
the
reduce-impl
branch
from
4e6768b to
1f122b8
Compare
loganchien
approved these changes
Nov 15, 2023
GleasonK
requested changes
Nov 16, 2023
|
The semantics LGTM. Have a few suggestions to make the spec more readable. |
sdasgup3
force-pushed
the
reduce-impl
branch
from
1f122b8 to
a8b8a99
Compare
GleasonK
reviewed
Nov 20, 2023
sdasgup3
force-pushed
the
reduce-impl
branch
2 times, most recently
from
015f1d3 to
67fa1f1
Compare
loganchien
approved these changes
Nov 28, 2023
paulinesho
approved these changes
Nov 28, 2023
GleasonK
approved these changes
Nov 29, 2023
…_norm_grad, batch_norm_training
sdasgup3
added a commit
that referenced
this pull request
Dec 20, 2023
Implements the specification changes at #1796. The PR adds/updates the verifier and type inference routines for the following ops: `reduce, reduce_window, select_and_scatter, all_reduce, reduce_scatter, scatter`. Please refer to #1796 for the updated constraints which the PR implements. Note the #1796 is going to be merged soon. Here are the changes for each operation: - reduce - #1796 added a new constraint C8 - Updated labels - Add positive tests and negative tests verifying reduce_c6 at verify_reduce.mlir and type inference tests at infrer_stablehlo.mlir, for reduce_c8. - reduce_window: - #1796 updated the C16 - Add positive tests ; negative tests verifying reduce_window_c13 at verify_reduce_window.mlir and type inference tests at infrer_stablehlo.mlir, for reduce_window_c16. - select_and_scatter - #1796 added a new constraint C12 - Updated labels - Add positive tests ; negative tests verifying selelct_and_scatter_c10 at verify_select_and_scatter.mlir and type inference tests at infrer_stablehlo.mlir, for select_and_scatter_c12. - scatter - #1796 added a new constraint C17 - Updated labels - Add positive tests ; negative tests verifying scatter_c15 at verify_scatter.mlir and type inference tests at infrer_stablehlo.mlir, for scatter_C17. - reduce_scatter - #1796 added a new constraint C9 - This op does not have the type inference supported. We had a trait `SameOperandsAndResultElementType` implementing the outdated constraint. For the new constraint C9, we added a check at `verifyReduceScatterOp`. - Add positive tests; negative tests verifying reduce_scatter_C7 at ops_stablehlo.mlir and type inference tests at ops_stablehlo.mlir, for reduce_scatter_C9. - all_reduce - #1796 added a new constraint C7 - This op implemented an outdated constraint related to type inference using `inferReturnTypeComponentsFromOperands`. For the new constraint C9, we added a trait `InferTensorType` in the tablegen definition of the op. - Updated labels - Add positive tests; negative tests verifying all_reduce_C5 at ops_stablehlo.mlir and type inference tests at ops_stablehlo.mlir, for all_reduce_C7.
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.
The PR implements the approved RFC for reduce op by proposing the specification related changes for
reduce,reduce_windowandselect_and_scatterops.In #1647, we talked about some of the other ops which will depend on the quantized specification of reduce op. Initially, I thought about creating separate PRs for them, but for the interest of time and the fact that their handling is going to be very similar to how
reduceop is handled, I propose to include their PR in the current PR.Here are the additional ops (other than
reduce,reduce_window,select_and_scatter) whose specification is addedOps with explicit computation regions:
all_reduce,reduce_scatter,scatter: They are handled similar to howreduceop is handled.Ops w/o explicit computation region:
batch_norm_grad,batch_norm_training: For these ops, the semantics of the operation implicitly does reduction with a custom computation function. As there is no explicit computation function in the IR, the proposal in the RFC cannot be applied. I propose (A) to handle these ops similar to howbatch_norm_inferenceis handled withdequant-op-quantstrategy, (B) we can revisit this op later if there is use case to do that implicit reduction using higher accumulation type.One implementation detail: The fact that
batch_norm_grad,batch_norm_trainingops returns multiple outputs and currentdequantize_op_quantizereturns a single output, is handled using a dedicated meta function for these two ops.Next steps: Once the PR is approved the plan is to propose the corresponding changes in the verifier/shape functions for these ops.
Only support promotion of reduction operands to reduction-block arguments
The specific question bought up in the discussion is: should we promote demotion as well? It seems that there aren't any use-cases for that. For example, consider the following cases of implicit conversion for reduction operations. The examples are shown using integer type but can be generalized to other types as well.
With that in mind, we propose to develop the specification based on the scenario we have the use-case for (just for promotion: none of the cases of demotion or extra promotion will be supported in the specification.