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

Support more ops in .tflite format #25296

Open
CNOCycle opened this issue Mar 29, 2024 · 4 comments
Open

Support more ops in .tflite format #25296

CNOCycle opened this issue Mar 29, 2024 · 4 comments

Comments

@CNOCycle
Copy link
Contributor

CNOCycle commented Mar 29, 2024

Describe the feature and motivation

Hi,

I would like to bring to your attention some recent developments regarding the implementation of more ops in .tflite format. Specifically, I've been working on incorporating global pooling, transpose, softmax, and fullyconnected ops. I noticed that the support for the last two ops has been merged in PR(#25273).

However, I have a couple of concerns regarding this PR. Firstly, in parseFullyConnected, FC ops are implemented using Gemm. In my view, Gemm and Fully_connected represent distinct concepts. The latter allows for more options, such as activation and bias terms. It appears that some functionalities are missing in the current implementation.

Secondly, the support for Softmax seems to be ambiguous across different formats. Specifically, while the Softmax layer in Keras and ONNX can accept axis as an argument, this option is absent in the .tflite format. The ambiguity arises from the default value of axis, as described in PR (#24613). Moreover, as shown in below, the default values differ between the FP version and the INT8 version.

axisRaw = params.get<int>("axis", -1);

axis = params.get<int>("axis", 1);

From my perspective, I believe setting the default value to -1 would align better with TF and ensure consistency. However, it might be beneficial to include comments regarding the default values somewhere to assist users in quickly diagnosing unexpected behavior, especially if the model originates from ONNX or other converters.

Additional context

No response

@fengyuentau
Copy link
Member

So what do you want?

@dkurt
Copy link
Member

dkurt commented Mar 31, 2024

@CNOCycle, let's start with tests. Please provide generation steps similar to https://github.com/opencv/opencv_extra/blob/4.x/testdata/dnn/tflite/generate.py or share the .tflite models so we can reproduce.

Please format issue with check boxes for each case to estimate the progress:
- [ ] SoftMax axis support

TFLite support is relatively new so any contribution are welcome! I see that you've started #25297 so let's go further reviewing it.

@CNOCycle
Copy link
Contributor Author

CNOCycle commented Apr 1, 2024

Hi @fengyuentau,

I would like to highlight a couple of points regarding this issue:

  • In my view, it is crucial to discuss the appropriate approach for handling the absence of the axis option in Softmax. As mentioned in the above, this seems to be causing confusion in the tflite model.
  • we need to decide between implementing the FC layer using either Gemm or Fully_connected .

If the opencv team is satisfied with the current implementation, no further action is needed, and I will proceed to close this issue.

@fengyuentau
Copy link
Member

we need to decide between implementing the FC layer using either Gemm or Fully_connected .

Our plan is to replace FullyConnected with Gemm for all importers for better performance. Gemm layer have already supported bias. Fusion of activation can be also added to Gemm layer.

So your feature checklist is

  • SoftmaxInt8 axis defaults to 1;
  • Support of activation fusion in Gemm layer.

Am I correct?


Also noticed from @dkurt 's comment that you have started contribution on Transpose operator support. We welcome contributions and you are welcome to contribute on the two listed items above.

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

4 participants