rust icon indicating copy to clipboard operation
rust copied to clipboard

ops::split() shorthand function is broken

Open Corallus-Caninus opened this issue 3 years ago • 4 comments
trafficstars

I cannot construct a split operation. when given the appropriate types in ops::split() the attribute num_split does not get assigned. I'd reference the line but the file is too large for github, in my IDE it is 111704 in src/ops/ops_impl

there seems to be a discrepancy between num_split and split_dim as well.

It also doesnt seem like a great idea to have if statements for build_impl codegen where those attributes are mandatory without a default value for the node but I am still learning the codebase.

If anyone sees this please let me know how I can help otherwise I'll wing it.

Corallus-Caninus avatar Apr 15 '22 01:04 Corallus-Caninus

I was able to work around this by building the operation without the helper function split() by doing the following:

let split_outputs = ops::Split::new()
    .num_split(num_splits)
    .build(axis_zero, input, scope)?;

where axis_zero is ops::constant(0,scope); and num_splits is ops::constant as well

however this doesnt close this issue since the helper function is broken and in the best case misleading.

Corallus-Caninus avatar Apr 17 '22 04:04 Corallus-Caninus

~~The gerated code is not broken, but default values are still missing as you pointed out. We may need a help to fix it.~~

Edited : Default value is not irrelevant.

dskkato avatar Apr 18 '22 08:04 dskkato

FYI ops code was generated from this definition file. https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/ops/ops.pbtxt

And the code was generated by tensorflow-op-codegen crate in this repository.

dskkato avatar Apr 18 '22 08:04 dskkato

Here is the Split definition in the pbtxt.

op {
  name: "Split"
  input_arg {
    name: "split_dim"
    type: DT_INT32
  }
  input_arg {
    name: "value"
    type_attr: "T"
  }
  output_arg {
    name: "output"
    type_attr: "T"
    number_attr: "num_split"
  }
  attr {
    name: "num_split"
    type: "int"
    has_minimum: true
    minimum: 1
  }
  attr {
    name: "T"
    type: "type"
  }
}

In this case, it seems we should not discard has_minimum and minimum field. Another cases would be has_maximum/maximum.

dskkato avatar Apr 20 '22 08:04 dskkato