[Feature request] checking an input rank is within a specific range

[ZelboK]

What is the problem that this feature solves?

Please keep in mind I am new to ONNX. I will be missing context on priorities with the code so this might be useless.

While looking into extending Microsoft's ORT functionality to accept a 5D input for Grid Sampling, I noticed it might be helpful to have shape inferencing capabilities to check an input's rank is within a range when you know the inputs rank ahead of time.

Currently shape_inference.h has

inline void checkInputRank(InferenceContext& ctx, size_t input_index, int expected_rank) {
  // We check the rank only if a rank is known for the input:
  if (hasInputShape(ctx, input_index)) {
    auto rank = getInputShape(ctx, input_index).dim_size();
    if (rank != expected_rank) {
      fail_shape_inference("Input ", input_index, " expected to have rank ", expected_rank, " but has rank ", rank);
    }
  }
}

which will work for only one rank. But if you want to extend an operators functionality to work within a certain range of ranks I believe it would be helpful to have an overload that will accept a range instead.

Alternatives considered

downstream code can use their own implementation by reusing functions like hasInputShape, getInputShape and fail_shape_inference.

Describe the feature

if it makes sense for the operator to work with different ranks, downstream code will not need to define their own function.

Will this influence the current api (Y/N)?

no

Feature Area

shape_inference

Are you willing to contribute it (Y/N)

Yes

Notes

I understand this is quite small and insignificant. Figured it was a good entry point to get to contributing to ONNX.

[justinchuby]

I think GridSample has been updated to support ND inputs (https://onnx.ai/onnx/operators/onnx__GridSample.html). To help me understand better, was your intention to support a range of input ranks during shape validation? What would this enable?

[ZelboK]

I think GridSample has been updated to support ND inputs (https://onnx.ai/onnx/operators/onnx__GridSample.html). To help me understand better, was your intention to support a range of input ranks during shape validation? What would this enable?

Yes your understanding is correct.

https://github.com/microsoft/onnxruntime/blob/3fb8905393bf91d97eae7afa0ae341828e3f1dbc/onnxruntime/core/graph/contrib_ops/contrib_defs.cc#L1044

If you look here, the logic is that it'll throw an error if you try to pass it input with anything but 4 dimensions. ORT is interested into extending this feature to support 5D, so (if my understanding is correct), this will need to change a bit to account for both 4D and 5D being valid inputs. If an operator is interested in supporting ND, would it not be helpful to have this validation?

Keep in mind this is a really small detail and I figured it was worth making a PR. I am still learning both ORT and ONNX - this isn't required functionality but I was mostly curious to see if it offered any utility. If I'm missing some context, please lmk and I can close both this PR and issue.

[justinchuby]

Thanks for the clarification. The new grid sample is going to support dimensions >5 as well, at least according to the spec. So specifying a range seems to be runtime specific. I would recommend adding this function only when it is needed for inferring shapes for onnx domain operations defined in this repository, to keep the APIs lean. Please correct me if there's anything I overlooked.

[ZelboK]

Thanks for the clarification. The new grid sample is going to support dimensions >5 as well, at least according to the spec. So specifying a range seems to be runtime specific. I would recommend adding this function only when it is needed for inferring shapes for onnx domain operations defined in this repository, to keep the APIs lean. Please correct me if there's anything I overlooked.

Sounds good, I don't think you've missed anything. Would you prefer this PR closed then?

(Tangential) can you point to some cxx related issues I can contribute towards if any exist? :)

[justinchuby]

Sounds good, I don't think you've missed anything. Would you prefer this PR closed then?

Yes. I think it's good to close it for now. Your contribution is appreciated still :)

(Tangential) can you point to some cxx related issues I can contribute towards if any exist? :)

Absolutely! Here is a list of issues that would welcome contributions: https://github.com/onnx/onnx/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3A%22contributions+welcome%22

Some c++ related ones are:

• Missing attribute in Split-18 not reported by checker #5222
• onnx.checker.check_model() does not report mismatch input/output shapes #5974
• Create down conversion for GroupNormalization-21 #5843
• Inliner fails on the attached model #5816

Etc.

[ZelboK]

Sounds good, I don't think you've missed anything. Would you prefer this PR closed then?

Yes. I think it's good to close it for now. Your contribution is appreciated still :)

(Tangential) can you point to some cxx related issues I can contribute towards if any exist? :)

Absolutely! Here is a list of issues that would welcome contributions: https://github.com/onnx/onnx/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3A%22contributions+welcome%22

Some c++ related ones are:

• Convert Split from opset11 to 18 failed. #5222
• onnx.checker.check_model() does not report mismatch input/output shapes #5974
• Create down conversion for GroupNormalization-21 #5843
• Inliner fails on the attached model #5816

Etc.

Much appreciated! I'll look into one of those :D