[RFC] Perform deeper function cloning. Some variables needs to be cloned.

[rdzhabarov]

Usage of cloning function

Glow currently provides a convenience method to clone an existing high level IR Function.
This functionality is used for procedures like:

• Profile quantization
• Quantization
• Differentiation

Cloning semantics

The semantics of cloning is to create a new Function with all nodes cloned (basically, creating a new nodes) and restore edges in the new Function based on the original one. Cloning does not involve cloning variables in any way, so that newly cloned Function reuses the same variables as the original one.

Multiple writers issue

At the moment of inference some of the variables are read only variables like weights while others (e.g., connected to the Save node) are not. Having Save nodes clonned while sharing variables creates a situation when results of inference on one function overwrites results of inference on another function. This feels like an unexpected behavior when normally cloning is done with copy on write or just cloning shared resource. Currently, quantization end to end test does not perform any sensible validation as second inference run overwrites original values.

Options to fix this

• The most natural way is to let Node cloning method to decide what needs to be cloned. Currently, it's just node itself, but for the case of Save node it could clone the output variable first and use that to create a new Save node.
• Let the cloning procedure to handle variables and clone some of them. This way cloning procedure needs somehow to know whether var is output var and clone only those.
• Other options?

@qcolombet, you looked at Save nodes a lot in the past. What do you think?

[qcolombet]

The most natural way is to let Node cloning method to decide what needs to be cloned. Currently, it's just node itself, but for the case of Save node it could clone the output variable first and use that to create a new Save node.

The problem I see with that is what if we indeed want the save node not to clone the Variable? For instance when we do the training of those variables.

In my opinion the variables should not be pinned to a location before we actually run (or lower) the program.
In other words, I believe we have such problem because of our representation of variables that we know is flawed. (See #1334)

Instead, I would recommend that we consider the variable as addresses, that would be inputs of the function (i.e., provided at a later stage, runtime or at lowering). As such, we would need to clone them like any regular node. It then becomes easy to share or split the write of a variable: it is just a matter of providing the same or a different address to the related function.

I.e., currently we have:

Variable global
Variable weight
fct1() {
read weight
save global
}

// cloned from fct1
fct1_cloned() {
read weight  // <-- shared weight
save global // <-- potentially problematic write
}

What I am suggesting:

fct1(Variable global, Variable weight) {
read weight
save global
}

// cloned from fct1
fct1_cloned(Variable global_cloned, Variable weight_cloned) {
read weight_cloned
save global_cloned
}

Then lowering or runtime can chose to bind global and global_cloned to the same address. Likewise for weight and weight_cloned.

IIRC this is pretty much what AOT does, so we could reflect that in the IR.

[rdzhabarov]

The problem I see with that is what if we indeed want the save node not to clone the Variable? For instance when we do the training of those variables.

Does that really impact training part? Computation graphs typically have Save nodes to store the outputs of the network. Differentiation would create a new Function with necessary SGD and Save nodes to write to trainable vars, not outputs. And the differentiated graph could be trained normally.

Irrespective of impact on training, I think that below makes sense and would fix other problems.

In other words, I believe we have such problem because of our representation of variables that we know is flawed.
Instead, I would recommend that we consider the variable as addresses, that would be inputs of the function (i.e., provided at a later stage, runtime or at lowering). As such, we would need to clone them like any regular node

[qcolombet]

Differentiation would create a new Function with necessary SGD and Save nodes to write to trainable vars, not outputs.

Good point, the variables we train wouldn't be outputs!

Given, it is likely it will take some time to do the "proper fix", going with your option #1 in the meantime works for me.

[rdzhabarov]

This was resolved by having input/output placeholders and multiple contexts.