One: [luci-interpreter] Interface to access tensors in preOperatorExecute and postOperatorExecute

[jinevening]

luci-interpreter supports hooks called before/after an operator is executed (preOperatorExecute and postOpeartorExecute). Currently, both hooks have only one parameter const luci::CircleNode *, as follows.

virtual void preOperatorExecute(const luci::CircleNode *node);
virtual void postOperatorExecute(const luci::CircleNode *node);

This interface allows very limited use cases, because we cannot access the contents of tensors by using CircleNode.

To allow the access of tensor values inside preOperatorExecute/postOperatorExecute, I'd like to add one more parameter (node_to_tensor) as follows.

virtual void preOperatorExecute(const luci::CircleNode *node, std::unordered_map<const loco::Node *, Tensor *> &node_to_tensor);
virtual void postOperatorExecute(const luci::CircleNode *node, std::unordered_map<const loco::Node *, Tensor *> &node_to_tensor);

[jinevening]

@s-barannikov PTAL. I think this may change the interface of many classes in luci-interpreter (especially ~~Loader).

[seanshpark]

If node_to_tensor doesn't depend on node then how about adding something like preNetworkExecute to pass node_to_tensor once before all the operators?

[jinevening]

how about adding something like preNetworkExecute to pass node_to_tensor once before all the operators?

This is a good idea. AFAIK, node_to_tensor is not updated after it is created. One concern is that it is still valid even with dynamic tensors.

[jinevening]

I think introducing preNetworkExecute and postNetworkExecute is a good idea. Currently users cannot access the input/output of the model (pre/postOperatorExecute is not called for CircleInput and CircleOutput).

The parameters of pre/postNetworkExecute would be the inputs of a model (a vector of CircleInput) and the outputs of a model (a vector of CircleOutput), respectively.

[s-barannikov]

@jinevening
Instead of adding the map to the parameters of preOperatorExecute and postOperatorExecute I would add a new interface const void* Interpreter::getTensorData(const luci::CircleNode *), which can be called from the user's implementation of preOperatorExecute / postOperatorExecute. This also aligns with my earlier proposal to remove the Tensor parameter from postTensorWrite method.

I think preNetworkExecute / postNetworkExecute are not necessary. They will be called just after entering and before exiting interpreter.interpret method, so the user already has information about when the network started / ended its execution.

[jinevening]

I would add a new interface const void* Interpreter::getTensorData(const luci::CircleNode *), which can be called from the user's implementation of preOperatorExecute / postOperatorExecute

Ok. I'll look forward to your PR for getTensorData. One question. Why not Interpreter::getTensorData returns Tensor? If the interpreter returns just void*, users will have to get the type and the shape of the tensor from luci::CircleNode to use the tensor data.

I think preNetworkExecute / postNetworkExecute are not necessary. They will be called just after entering and before exiting interpreter.interpret method, so the user already has information about when the network started / ended its execution.

I understand. Thanks.

[s-barannikov]

Why not Interpreter::getTensorData returns Tensor?

I would like to remove Tensor from interpreter's interface. It is a low-level detail, which should not be exposed. But...

If the interpreter returns just void*, users will have to get the type and the shape of the tensor from luci::CircleNode to use the tensor data.

is a good point. The thing is - luci::CircleNode may not have valid shape, and void * does not event tell the size of the data.

Let's make it return Tensor for now.

[jinevening]

I close this PR as #3055 was merged.