blueoil icon indicating copy to clipboard operation
blueoil copied to clipboard

Introduce new class to manage tensors

Open primenumber opened this issue 4 years ago • 4 comments

In current implementation of converter, parameters (shape, dtype, ... or dimension) of tensor are member of Operator class, but Split operator can have multiple outputs. Because of that implementation, Split operator (and other operators) has inconvenient limitations.

primenumber avatar May 28 '20 00:05 primenumber

Current: each Operator has input_ops: Dict[str, Operator], output_ops: Dict[str, List[Operator]]

With Tensor class: each Operator has input_tensors: Dict[str, Tensor], output_tensors: Dict[str, Tensor] each Tensor has source: Operator, destinations: List[Operator], but these properties are read only (calculated from Operator 's properties).

primenumber avatar May 28 '20 01:05 primenumber

@tkng How do you think to add new classes for Tensor.

iizukak avatar Jul 01 '20 01:07 iizukak

As a long term vision, I think introducing a new class for Tensor is a good idea. On the other hand, it might be possible to implement Split operator by duplicating operator node (in that case, actual implementation will be similar to Slice operator, rather than Split operator). Hence, if the problem is only in the Split operator, it might be possible to avoid the problem with this idea.

I think the issue is which idea less costly in the short term. If the implementation cost of Tensor class is lower, then there is no reason to not implement Tensor class. On the other hand, if improvement of Split operator is could be done by lower implementation cost, it might be better to choose the latter idea.

Again, as a long term vision, Tensor class would be a good idea.

@primenumber Which do you think is less expensive to implement?

tkng avatar Jul 01 '20 03:07 tkng

@tkng Oh, The implementation cost of duplicating operator sounds reasonable. I'll try it first.

primenumber avatar Jul 01 '20 05:07 primenumber