onnx: [Operator] Why Cumsum axis is not an attribute?

Hi, I’m wondering why Cumsum op, introduced in https://github.com/onnx/onnx/pull/2030, requires axis to be passed as input, rather than being an attribute? Other ops that take an axis, such as Reduce*, Scatter, Gather, Split, Concat etc all seem to take axis as an attribute. Also in practice, I cannot imagine a use case where we want to have a variable axis for cumsum.

Unless there is a good reason to have a variable axis for Cumsum, I believe this is a flaw in the spec that needs to be corrected.

cc @jcwchen @postrational @wschin

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 15 (5 by maintainers)

Most upvoted comments

Yes I generally agree with moving toward dynamic parameters, provided there is a good reason to do so for each param. In that sense, I think dynamic topk param makes sense, while I don’t see why anyone would want to have dynamic axes.

So I’m interested to hear the motivation of https://github.com/onnx/onnx/pull/2955, in particular why the need to “Support dynamic shapes in ORT” (good) leads to the decision to add dynamic axes support in certain but not all ops (confusing).

Could TVM apply the same logic at compile time, and treat all const cases equivalently?

Yes, we can do the same, if axis parameter is provided as a constant. We can remove the argument vs attribute distinction by our ONNX frontend. But our cumsum op by itself expects axis as a compile time constant, so we would have a trouble if axis is truly a dynamic variable. I think we have the same concern here.