ONE icon indicating copy to clipboard operation
ONE copied to clipboard

[onert] Support multiple model nnpkg (tests/nnfw_api)

Open glistening opened this issue 2 years ago • 5 comments

I would like to add tests for multiple model nnpkg.

Currently nnfw_api tests are located in tests/nnfw_api, and it can only handle single model nnpkg. We need to define how to add tests for multiple model nnpkg.

Option 1: Add multiple model nnpkg as we did

We can add multiple model nnpkg as https://github.com/Samsung/ONE/tree/master/tests/nnfw_api#nnpackages-for-testing.

Option 2: Don't add nnpkg, but generate nnpkg during test.

NOTE It is not recommended adding a test this way, since you can make a Circle model with some code using CircleGen class.

Some members don't like to keep tests in file. They seem to like create model by API each test execution.

Then, we may generate nnpkg using file/directory creating during test execution. Especially for testing frontend (which is my interest now), it is enough to create MANIFEST and dummy model files.

@Samsung/one_onert, @hseok-oh I would like to hear your opinions.

I like Option 1. (Simple and almost zero additional effort to generate the existing test in C/C++ code).

glistening avatar Jul 05 '22 01:07 glistening

I prefer Option2. But I concern that how generate trix binary during test execution when testing with trix binary.

chunseoklee avatar Jul 05 '22 04:07 chunseoklee

As you commented, CircleGen is introduced to prevent including model file in this repo and remove dependency with server setting. Option 1 is used to check nnpackage unittest to check new nnpackage feature, so we use small example package in nnpackage/examples, and it is dependent with server environment. If you want small unittest to check validation about new nnpackage feature, we may use Option 1 after we add example nnpackage for new feature - multiple model nnpkg.

Different with this issue)

When I investigate about this issue from above document, I found mobilenet tflite binary is in our repo. (runtime/contrib/TFLiteSharp/TFLiteTestApp/res/mobilenet_v1_1.0_224.tflite). Maybe nobody know about this because TFLiteSharp contrib is very old project, and imported from internal repo PR no 1486.

And this file is used on nnfw_api test for pipeline. I didn't check about this. It is unexpected usage.

hseok-oh avatar Jul 05 '22 04:07 hseok-oh

I prefer Option2.

May I ask why? The same reason @hseok-oh mentioned? ( = to prevent including model file in this repo and remove dependency with server setting. )

But I concern that how generate trix binary during test execution when testing with trix binary.

As of now, for frontend test, trix model is not necessary. We can use two circle (or tflite) models in nnpkg with pkg-inputs, pkg-outputs and model-connect.

I don't think we can add trix model in this repo.

glistening avatar Jul 05 '22 05:07 glistening

As you commented, CircleGen is introduced to prevent including model file in this repo and remove dependency with server setting. Option 1 is used to check nnpackage unittest to check new nnpackage feature, so we use small example package in nnpackage/examples, and it is dependent with server environment. If you want small unittest to check validation about new nnpackage feature, we may use Option 1 after we add example nnpackage for new feature - multiple model nnpkg.

Different with this issue)

When I investigate about this issue from above document, I found mobilenet tflite binary is in our repo. (runtime/contrib/TFLiteSharp/TFLiteTestApp/res/mobilenet_v1_1.0_224.tflite). Maybe nobody know about this because TFLiteSharp contrib is very old project, and imported from internal repo PR no 1486.

And this file is used on nnfw_api test for pipeline. I didn't check about this. It is unexpected usage.

This approach looks good for positve test, but for negative test, it seems not natural to add wrong example in nnpackage/examples.

Let's think more to find a solution that works for backends tests and negative tests.

glistening avatar Jul 05 '22 05:07 glistening

Let me write down summarize current status.

  • Two tflites (merged in this repo)
    • https://github.com/Samsung/ONE/tree/master/nnpackage/examples/v1.3.0/two_tflites
  • One circle + One tvn (available in internal repo because of tvn)
  • For negative test
    • We may generate an NNPkg in gtest when it become necessary.

glistening avatar Sep 05 '22 06:09 glistening

For positive test, we've done with option 1. Let me create a new issue when negative tests become necessary.

glistening avatar Nov 10 '22 03:11 glistening