Adding EyeLike generator op. #1428
Conversation
spandantiwari
requested review from
houseroad,
linkerzhang,
ebarsoum and
prasanthpul
spandantiwari
force-pushed
the
spandantiwari/eyelike_op
branch
from
9a36916
to
5d43904
Compare
|
All CI checks are passing. Friendly ping to get a review. |
onnx/defs/generator/defs.cc
Outdated
| .TypeAndShapeInferenceFunction([](InferenceContext& ctx) { | ||
| if (ctx.getAttribute("dtype") != nullptr) | ||
| propagateElemTypeFromAttributeToOutput(ctx, "dtype", 0); | ||
| else |
There was a problem hiding this comment.
Nit: add {} even if there is only one stmt.
| OPTIONAL) | ||
| .Input( | ||
| 0, | ||
| "input", |
There was a problem hiding this comment.
why not have number of rows and columns as parameters rather than infer it from an input tensor? numpy, pytorch and mxnet all have explicit rows and cols as parameters.
There was a problem hiding this comment.
I think we can have another operator called Eye which takes rows and columns as inputs.
There was a problem hiding this comment.
won't that be like duplicating the functionality? are there examples where this Eyelike operator is used?
There was a problem hiding this comment.
Yes, that's the idea. The *Like ops take the properties such as shape and type from the input tensor. This design is similar to Numpy (ones_like, zero_like) and also other ops in ONNX such as ConstantLike, RandomNormalLike and RandonUniformLike.
We can have a separate PR for Eye if needed.
There was a problem hiding this comment.
@anirudhacharya - I don't think this is exactly duplicating functionality. There are several frameworks that subscribe to the design where there are two parallel set of generator ops - one that take tensor as input and one that take explicit shape as input, e.g. (ones, ones_like), (zeros, zeros_like). This kind of op is quite useful, e.g. creating epsilon covariance matrices matrices for regularization, and matrix factorization scenarios.
Also, note that I have not added Eye in this PR in order avoid adding more ops than necessary, because something like Eye, with static shape, can be supported using Constant op in current spec.
spandantiwari
force-pushed
the
spandantiwari/eyelike_op
branch
from
29680cf
to
8c2241a
Compare
|
Updated the PR based on the feedback. |
|
@houseroad , @anirudhacharya - Thanks for the review. |
|
@spandantiwari : Would it be useful to keep the 2D restriction (I can't think of any use case extending identity matrices to 3D/4D given matmul's are 2D anyway) but treat any higher dimensions as a batch count? I mean, if this operator is useful for processing one image, then it should be useful for processing a batch of images in parallel like MatMul and Conv2D (without Concat'enating a bunch of EyeLike outputs). I'm asking because GPU's are better at processing a lot of similar data in parallel than repeating lots of smaller varied executions. |
|
Hello. I am new to ONNX and I am trying to understand how to add operators. |
This is a generator ops for producing identity-like matrices with ones on the major diagonal and zeros everywhere else. This generator op is quite useful for models and most major frameworks have it.
Note that the input is restricted to 2D because that is the most dominant use case and most of the frameworks have support restricted to 2D for this (similar) op.