One: [Compiler] Handling optional input tensor

[llFreetimell]

Related Issue or PR

Draft PR : TBD
Candidate solution 1 shared : #643

Status

There are some cases that an operator has no input tensor. For example, bias of FullyConnected is optional tensor and it can be none. Following graph is an example.

    input1 (input) ------ FullyConnected ---- output1
    input2 (weights) --+
    NONE (bias) -------+

For now, tensor index of those empty tensors is -1 and it causes some errors from below codes.
https://github.com/Samsung/ONE/blob/90c92dba99ad9c403de93292b642dad69bc03481/compiler/luci/import/src/GraphBuilder.cpp#L33

Candidate Solutions

• Introducing NoOp
  If we insert NoOp for those empty tensors and add some codes to handle the NoOp at each operator codes, we can handle it.

[Original Model File]
input1 (input) ------ FullyConnected ---- output1
input2 (weights) --+
NONE (bias) -------+

[loco::Graph after imported by luci]
input1 (input) ------ FullyConnected ---- output1
input2 (weights) --+
NoOp (bias) -------+

[Circle model after circle2circle finished]
input1 (input) ------ FullyConnected ---- output1
input2 (weights) --+
NONE (bias) -------+

However, it can be burden to implement all the each cases at each operators.
_NOTE_ NoOp is a virtual operation which does anything, and the name can be changed.

• Handle each cases at each operator
  We can handle this issue at each operator. For example, in case of FullyConnected, we can implement two cases. The one is when FullyConnected has two input tensors and the other is when FullyConnected has three input tensors.
  However, it can be sometimes hard if there are many optional input tensors like LSTMCell.
  
  
  
  • Using Variadic Input
  
  

[seanshpark]

Can you describe more for what before, import, after is about?
Is before a Circle file before and import for IR tree after import and then after an Circle file after export?

[llFreetimell]

@seanshpark
Yes, as you said, I wanted to indicate following steps.

.circle ------> circle2circle ------> .circle
[before]        [import]              [after]

before : Original model, which has no input tensor for optional input.
import : loco::Graph after imported by luci, which NoOp is inserted.
after : Circle model after circle2circle finished, which NoOp is not exported and same with original model.

I updated the contents.

[seanshpark]

OK. it would be better to chane [Original Model] to [Original Model File] to make it more specific.

[mhs4670go]

How about using VariadicArityNode if node has optional input. You can prevent nullptr from occurring in the input.

[s-barannikov]

Why don't just support optional inputs? E.g. pass nullptr if an input is not provided.

[llFreetimell]

@mhs4670go That is one of possible solutions :)

@s-barannikov Maybe this is what you said! Following codes are from #671, which is created by @mhs4670go :)
https://github.com/Samsung/ONE/blob/e49d6c0fad64ff41cdc4da873392b411232d27bd/compiler/luci/import/src/GraphBuilder.cpp#L31-L41

[llFreetimell]

_cf. NoOp is changed to CircleOutputExclude_

There were two options to handle this issue, first one was using CircleOutputExclude and the other one was using nullptr.
However, I want to handle it by CircleOutputExclude and the related PR is posted at #1010.
Followings are the reason why I choose first option.

• It can be very hard to distinguish whether nullptr is inserted by us or error made it
• When circle operation exported at CircleOperationExporter.cpp, we should insert tensor index after checking whether node->something() is nullptr or not every time. It causes more if statements.
• nullptr can cause some errors when shape inference or type inference. We may be able to handle this by implementing every cases but... it may be burden I think.

So, I will proceed by first option but thanks for all the opinions!

[s-barannikov]

It can be very hard to distinguish whether nullptr is inserted by us or error made it

Can you guarantee that CircleOutputExclude is not inserted by error?) Or that any other argument is correct? There are tests for this.

When circle operation exported at CircleOperationExporter.cpp, we should insert tensor index after checking whether node->something() is nullptr or not every time. It causes more if statements.

You don't have to do it every time, you can just add a function and handle it there.

nullptr can cause some errors when shape inference or type inference. We may be able to handle this by implementing every cases but... it may be burden I think.

Do you mean that optional inputs do not have types / shapes? It does not matter whether it is nullptr or CircleOutputExclude, the difficulties are the same :)

So, I will proceed by first option but thanks for all the opinions!

I'm not agains the chosen solution, it is fine for me. Except probably the naming is a little... awkward 😀 It is hard to tell from its name what it is supposed to do. Exclude which output? How does it interfere with optional inputs?

[llFreetimell]

@s-barannikov Thanks for your opinion :)

---

Can you guarantee that CircleOutputExclude is not inserted by error?) Or that any other argument is correct? There are tests for this.

Of course, CircleOutputExclude cause some errors. But my point was this.
Unless we create CircleOutputExclude explicitly, there are no cases that CircleOutputExclude is generated by accident, I thought.
On the other hand, nullptr can appear by many cases. For example, we may not inserted some correct parameters, some parameters are missed after luci::Pass finished, or nullptr is inserted because there were no assert like assert(something != nullptr);.
In conclusion, I can't guarantee that CircleOutputExclude is not inserted by error but we may expect the number of causes can be decreased at least :)

---

You don't have to do it every time, you can just add a function and handle it there.

Yes, I thought exactly same thing at first and it was possible either :-) However, to use nullptr, we should introduce new function because original function like set_tensor_index or get_tensor_index don't want to accept nullptr.
It seems not that different with adding a checking code everytime for me :(

---

Do you mean that optional inputs do not have types / shapes? It does not matter whether it is nullptr or CircleOutputExclude, the difficulties are the same :)

In some cases, the difficulties are same as you said :)
However, as we can use loco::NodeTrait::Datatype and loco::NodeTrait::TensorShape for CircleOutputExclude, some shape/type inferences can be done easily.
For example, in case of FullyConnected, bias can be none. If we insert CircleOutputExclude at the bias and annotate shape and type as [1] and TF.FLOAT32, we can use same shape/type inference logic. Then, we do not need to add some codes to handle each cases and difficulty is pretty low!

---

Except probably the naming is a little... awkward grinning It is hard to tell from its name what it is supposed to do.

Actually there were no perfect naming for it :( CircleOutputExclude means This Circle node will be excluded when exporting step thus not exported.

---

Exclude which output? How does it interfere with optional inputs?

It works as followings.

[Original Model File]
input1 (input) -------- FullyConnected ---- output1
input2 (weights) ----+
  X    (bias) -------+

[loco::Graph after imported by luci]
             input1 (input) --------- FullyConnected ---- output1
             input2 (weights) ----+
CircleOutputExclude (bias) -------+

[Circle model after circle2circle finished]
input1 (input) -------- FullyConnected ---- output1
input2 (weights) ----+
  X    (bias) -------+

If there are no tensor in optional input when we import circle file, we specify it using CircleOutputExclude. Then, when we export circle file, we will not export them as same as original model.
In short, CircleOutputExclude exclude itself, and do not interfere optional inputs.

---

Long comments!

[s-barannikov]

@llFreetimell Thank you for the long explanation and the pretty diagram! :)

In short, CircleOutputExclude exclude itself, and do not interfere optional inputs.

So the purpose of CircleOutputExclude is to exclude itself, and we need to mark absent inputs. For me it seems like two independent things :)

Actually there were no perfect naming for it :(

What about straightforward CircleAbsentInput? It clashes with CircleInput in some way, but I think it is still a better name.

[llFreetimell]

@s-barannikov Not that pretty :laughing:

---

So the purpose of CircleOutputExclude is to exclude itself, and we need to mark absent inputs.

Exactly! That's what I wanted to express :grinning:

What about straightforward CircleAbsentInput? It clashes with CircleInput in some way, but I think it is still a better name.

The first issue for CircleOutputExclude was #643. (This issue and #643 is still independent).
After I decided not to use nullptr, I needed something to mark absent inputs like CircleAbsentInput as you said.
However, I thought CircleOutputExclude can be used here too because it has a common thing as follow.

• If input of CircleOutput is CircleOutputExclude, we will exclude the CircleOutput when exporting.
• If input of optional input tensor is CircleOutputExclude, we will exclude the input tensor when exporting.

So, I didn't introduce a new IR like CircleAbsentInput even though your words make sense =)

---

Meanwhile, related PRs are merged :cry:
If the name causes some critical confusions or should be changed in the future, I will modify the name or introduce new IR like CircleAbsentInput later, if you don't mind... :(
I am sorry for not giving you enough time to discuss...

[s-barannikov]

@llFreetimell

Meanwhile, related PRs are merged 😢
If the name causes some critical confusions or should be changed in the future, I will modify the name or introduce new IR like CircleAbsentInput later, if you don't mind... :(
I am sorry for not giving you enough time to discuss...

It's OK, really. I'm fine as long as @seanshpark agrees with the changes

[seanshpark]

I will modify the name or introduce new IR like CircleAbsentInput later

OK with me :)

[llFreetimell]

@s-barannikov It should have been hard for you to read my loooong comments but you wrote good opinions! I appreciate it :smile: