Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break dependency of kernels on MicroGraph #2186

Closed
wants to merge 3 commits into from

Conversation

rascani
Copy link
Contributor

@rascani rascani commented Aug 24, 2023

The control flow and resource kernels make calls into MicroGraph by using the MicroContext::graph() accessor. For interpreter-less invoke, we don't need a full MicroGraph, only the subset of functionality that the kernels use. This PR makes MicroGraph an abstract base class, which MicroInterpreterGraph now implements. Existing MicroGraph functionality was all moved to MicroInterpreterGraph. The kernels only depend on MicroGraph, but the MicroContext will still have a full MicroInterpreterGraph.

BUG=b/295174086

@rascani rascani added the ci:run label Aug 24, 2023
@TFLM-bot TFLM-bot removed the ci:run label Aug 24, 2023
The control flow and resource kernels make calls into MicroGraph by
using the MicroContext::graph() accessor. For interpreter-less invoke,
we don't need a full MicroGraph, only the subset of functionality that
the kernels use. This PR introduces an abstract base class for
MicroGraphInfo, which MicroGraph implements. The kernels only depend on
MicroGraphInfo, but the MicroContext will still have a full MicroGraph.

BUG=b/295174086
@rascani rascani force-pushed the micro_graph_refactor branch from 2a5de6f to 4d83fab Compare August 24, 2023 22:08
@rascani rascani added the ci:run label Aug 24, 2023
@TFLM-bot TFLM-bot removed the ci:run label Aug 24, 2023
Instead of introducing a new base class called MicroGraphInfo, this
commit makes MicroGraph the abstract base class and moves the
implementation of MicroGraph to MicroInterpreterGraph. The kernel
changes were all reverted, as they'll use MicroGraph instead of
MicroGraphInfo. The Interpreter and related test infrastructure will all
use the MicroInterpreterContext class.
@rascani rascani requested a review from advaitjain August 29, 2023 22:48
@rascani rascani added the ci:run label Aug 29, 2023
@TFLM-bot TFLM-bot removed the ci:run label Aug 29, 2023
@rascani rascani marked this pull request as ready for review August 29, 2023 23:19
@rascani rascani requested a review from a team as a code owner August 29, 2023 23:19
#include "tensorflow/lite/schema/schema_generated.h"

namespace tflite {

// MockMicroGraph stubs out all MicroGraph methods used during invoke. A count
// of the number of calls to invoke for each subgraph is maintained for
// validation of control flow operators.
class MockMicroGraph : public MicroGraph {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to fix this to be dependent on the abstract type. I'm gonna try to fix that before landing.

@rascani
Copy link
Contributor Author

rascani commented Aug 31, 2023

Closing in favor of #2202

@rascani rascani closed this Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants