rust icon indicating copy to clipboard operation
rust copied to clipboard

Merge operation accepts only single input

Open SamKG opened this issue 2 years ago • 5 comments

The Merge operation (https://www.tensorflow.org/api_docs/cc/class/tensorflow/ops/merge) should accept a list of inputs, and output the first received tensor. However, the rust binding (https://tensorflow.github.io/rust/tensorflow/ops/fn.merge.html) currently accepts only a single input (despite the input variable being named "inputs"). Digging through the source code, I found that Merge.build_impl internally also calls nd.add_input instead of nd.add_inputs (https://tensorflow.github.io/rust/src/tensorflow/ops/ops_impl.rs.html#57909)

Is there a way to pass multiple inputs using this api?

SamKG avatar Jan 14 '22 17:01 SamKG

ref #325

dskkato avatar Jan 15 '22 03:01 dskkato

It looks like the problem is that we're ignoring number_attr on input_arg. This is an issue for any op that takes a list as an input.

adamcrume avatar Apr 24 '22 21:04 adamcrume

@adamcrume I'd be happy to take a stab at fixing this - any pointers on where to start? Is it as simple as using the add_inputs method I mentioned earlier?

SamKG avatar Apr 24 '22 22:04 SamKG

This wouldn't be quite that straightforward. The code for the ops is generated by tensorflow-op-codegen/src/main.rs, and I think the fix is to change write_short_fn, write_build_fn, and write_build_impl_fn so that they use a slice of Outputs rather than a single Output for any input_arg where number_attr is set. Then the ops would need to be regenerated by running cargo run -- $PATH_TO_TENSORFLOW $PWD/.. from the tensorflow-op-codegen folder.

adamcrume avatar Apr 25 '22 04:04 adamcrume

Does the below build_instance method close this?

https://tensorflow.github.io/rust/tensorflow/ops/struct.Merge.html

Corallus-Caninus avatar Oct 26 '22 01:10 Corallus-Caninus