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

[compiler FE/onert-micro] Support CircleGRU #12263

Open
1 of 2 tasks
BalyshevArtem opened this issue Dec 11, 2023 · 20 comments
Open
1 of 2 tasks

[compiler FE/onert-micro] Support CircleGRU #12263

BalyshevArtem opened this issue Dec 11, 2023 · 20 comments
Assignees

Comments

@BalyshevArtem
Copy link
Contributor

BalyshevArtem commented Dec 11, 2023

What

Let's support CircleGRU as custom circle operation in compiler FE and onert-micro.

Why

Some target models for onert-micro contains such pattern. This conversion accelerates the execution of target models in onert-micro.

How

@BalyshevArtem BalyshevArtem self-assigned this Dec 11, 2023
@BalyshevArtem
Copy link
Contributor Author

First draft to check impact #11840 of such conversion

@BalyshevArtem
Copy link
Contributor Author

Before conversion

Main graph:

main_g

Cond graph:

cond_g

Body graph:

body_g

@BalyshevArtem
Copy link
Contributor Author

After conversion:

opt_g

@seanshpark
Copy link
Contributor

Are you going to use custom_gru as the op type?
Op is CustomOp and custom code is custom_gru. This is odd.
I would be better to rename this to some meaningful name.
What kind of GRU is this? Plz add some details about this.

@seanshpark
Copy link
Contributor

@lemmaa , @hseok-oh , what do you think of adding new Circle Op CIR_GRU = -5 in circle schema?

  • CIR_ prefix is to avoid future conflict when GRU is added in tflite , just optional, not the main point.

@BalyshevArtem
Copy link
Contributor Author

BalyshevArtem commented Dec 12, 2023

Are you going to use custom_gru as the op type? Op is CustomOp and custom code is custom_gru. This is odd. I would be better to rename this to some meaningful name. What kind of GRU is this? Plz add some details about this.

Yes, use CustomOp with custom code - CUSTOM_GRU.

I would be better to rename this to some meaningful name

Smth like CIRCLE_CUSTOM_GRU?

What kind of GRU is this?

GRU operation that is obtained using tf.keras.layers.GRU(units=num_units, activation='tanh') and where return_sequences=False. After conversion from tf into tflite it has pattern like #12263 (comment)

After supporting such pattern we will support with return_sequences=True

@BalyshevArtem
Copy link
Contributor Author

@lemmaa , @hseok-oh , what do you think of adding new Circle Op CIR_GRU = -5 in circle schema?

It can be helpful for us.

@seanshpark
Copy link
Contributor

Smth like CIRCLE_CUSTOM_GRU?
GRU operation that is obtained using tf.keras.layers.GRU(units=num_units, activation='tanh')

How about CircleGru ?
(FYI #12242 (comment) for existing codes)

@BalyshevArtem
Copy link
Contributor Author

How about CircleGru ?

For me good. Is it for custom code, right?

@seanshpark
Copy link
Contributor

Is it for custom code, right?

Yes, as @lemmaa and @hseok-oh hasn't replied yet.

@lemmaa , @hseok-oh , PTAL

@lemmaa
Copy link
Member

lemmaa commented Dec 14, 2023

Is it for custom code, right?

Yes, as @lemmaa and @hseok-oh hasn't replied yet.

@lemmaa , @hseok-oh , PTAL

LGTM. This will be useful for GRU acceleration. :)

@hseok-oh
Copy link
Contributor

Is it for custom code, right?

Yes, as @lemmaa and @hseok-oh hasn't replied yet.

@lemmaa , @hseok-oh , PTAL

Good.

@seanshpark
Copy link
Contributor

@BalyshevArtem , we are good to go with new CircleGRU IR.
If you like, you can introduce this IR, or I can work on this (maybe a little bit faster to land).

@hseok-oh
Copy link
Contributor

hseok-oh commented Dec 15, 2023

@BalyshevArtem, @seanshpark In schema, does CircleGRU need CircleGRUOptions for OP's parameter?

@seanshpark
Copy link
Contributor

from the draft #11840 , I think there's no option, for some attributes, is needed.
but would be better to get confirm from @BalyshevArtem

@BalyshevArtem
Copy link
Contributor Author

@BalyshevArtem, @seanshpark In schema, does CircleGRU need CircleGRUOptions for OP's parameter?

Actually for future good to have, I think CircleGRUOptions have to contain these fields:

  1. fused_activation_function
  2. return_sequences - bool value
  3. time_major - bool value

@BalyshevArtem
Copy link
Contributor Author

from the draft #11840 , I think there's no option, for some attributes, is needed.

If we can obtain CircleGRUOptions than I will add in the draft it

@BalyshevArtem
Copy link
Contributor Author

@BalyshevArtem , we are good to go with new CircleGRU IR.
If you like, you can introduce this IR, or I can work on this (maybe a little bit faster to land).

Thank you, I will try to add it)

@seanshpark
Copy link
Contributor

@BalyshevArtem , please try with this in your draft (I think you already know this)

table GRUOptions {
  fused_activation_function:ActivationFunctionType;
  return_sequences : bool;
  time_major : bool;
}

@BalyshevArtem BalyshevArtem changed the title [luci-pass/onert-micro] Convert GRU pattern to CustomGRU [compiler FE/onert-micro] Support CircleGRU Dec 18, 2023
@chunseoklee chunseoklee self-assigned this Oct 7, 2024
@seanshpark
Copy link
Contributor

this is active

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants