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

Feature check is prohibiting calling ml-api from ml-train api side which is at another repo. #110

Closed
zhoonit opened this issue Nov 17, 2021 · 5 comments

Comments

@zhoonit
Copy link
Contributor

zhoonit commented Nov 17, 2021

Problem

ml_train_api(nntrainer) unittest fails after recent SR

Cause

  1. First call to ml_tensors_info_create(info) fails in below line(https://github.com/nnstreamer/nntrainer/blob/24a7738539cadd9d6ce5c0ee479eade373f8ea6f/api/capi/src/nntrainer.cpp#L1101-L1104)

  2. check feature fails at
    https://github.com/nnstreamer/api/blob/main/c/src/ml-api-common.c#L29

I am not sure how earlier versions made it possible to pass the given function available at gbs at this point.

Emergency measures

  • call _ml_set_feature_check from nntrainer side to turn the feature check off.

int _ml_tizen_set_feature_state (int state);

emergency PR will be updated soon. nnstreamer/nntrainer#1715

Long term solutions (maybe more)

  1. systematical approach to control features inside gbs (I heard that this is probably not viable from @jaeyun-jung though :p)
  2. testing outside of gbs
  3. migrate (at least test) to ml api (maybe this should not be considered as a long term solution because it still pose problem if someone is to use ml-api inside gbs anyway)
  4. Having a package to control a configuration which has a feature control (mimicking system_level feature check)
@taos-ci
Copy link
Collaborator

taos-ci commented Nov 17, 2021

:octocat: cibot: Thank you for posting issue #110. The person in charge will reply soon.

@zhoonit
Copy link
Contributor Author

zhoonit commented Nov 18, 2021

Pulling comment here @jaeyun-jung
nnstreamer/nntrainer#1712 (comment)

Need discussion later.
To run the unittests on gbs-build, we should block tizen-feature for ML API. Internally ml-api (api repository, nnstreamer only now) disables feature state while running unittests.
If nntrainer uses ml-common APIs (tensors-info and tensors-data), unittests in nntrainer always report failure because common ML API cannot check the feature state on gbs-build.

@myungjoo
Copy link
Member

You may keep using _ml_set_feature_check for unit-test. You are not doing integration test; thus, you are not supposed to test other operating system (Tizen) features with your unit tests.

_ml_set_feature_check is not a workaround for unit-test, although you are NOT supposed to use it in integration test or tests in an actual target devices.

Comments on the "Long term solutions"
1: no. you don't need to do so.
2: yes, for Tizen, you need to do so for integration tests (there are such tests in dashboard.tizen.org, conducted when you do SR). But, keep in mind that this is a different and independent test set and you should keep the current "conventional" unit tests.
3: no. you are going to test tests, not the core implementation.
4: no. you are over-engineering. turning on/off feature checks is sufficient.

If you want to test feature-checks correctly, you need to add integration test cases (look at "ITC" of Tizen)

@zhoonit
Copy link
Contributor Author

zhoonit commented Nov 18, 2021

Cool, thanks for the comment!

As per comment, I guess nnstreamer/nntrainer#1715 is sufficient to close this issue :)

You may keep using _ml_set_feature_check for unit-test.

@jaeyun-jung could you once review nnstreamer/nntrainer#1715 for the completeness?

Thank you!

@zhoonit
Copy link
Contributor Author

zhoonit commented Nov 19, 2021

Resolved with nnstreamer/nntrainer#1715 :)

@zhoonit zhoonit closed this as completed Nov 19, 2021
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

No branches or pull requests

3 participants