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

[ unittest ] Fix modelfile TCs @open sesame 11/07 16:09 #2788

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

skykongkong8
Copy link
Member

@skykongkong8 skykongkong8 commented Nov 7, 2024

  • backbone_relative_to_ini_p : fix invalid bn layer param
  • backbone_08_n : insufficient init parameters should fail, thus wrong test status
  • backbone_09_n : current loadFromConfig does not return ML_ERROR_NOT_SUPPORTED. Think this is deprecated. Instead, ML_INVALID_PARAM is thrown in current version
  • buffer_size_smaller_than_batch_size_p, buffer_size_smaller_than_batch_size_2p : missing res directory
  • basic_dataset_p, basic_dataset2_p, basic_dataset3_p: missing res directory

Self evaluation:

  1. Build test: [X]Passed [ ]Failed [ ]Skipped
  2. Run test: [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: skykongkong8 [email protected]

- Remove todo doxygen comments for resolved test cases that have been temporally commented-out

**Self evaluation:**
1. Build test:     [X]Passed [ ]Failed [ ]Skipped
2. Run test:     [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: skykongkong8 <[email protected]>
- backbone_relative_to_ini_p : fix invalid bn layer param
- backbone_08_n : insufficient init parameters should fail, thus wrong test status
- backbone_09_n : current loadFromConfig does not return 'not supported'
- buffer_size_smaller_than_batch_size_p, 2p : missing res directory
- basic_dataset_p 1,2,3 : missing res directory

**Self evaluation:**
1. Build test:     [X]Passed [ ]Failed [ ]Skipped
2. Run test:     [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: skykongkong8 <[email protected]>
@taos-ci
Copy link
Collaborator

taos-ci commented Nov 7, 2024

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2788. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

@skykongkong8 skykongkong8 changed the title [ unittest ] Fix modelfile TCs [ unittest ] Fix modelfile TCs @open sesame 11/07 13:26 Nov 7, 2024
Copy link
Collaborator

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

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

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

@skykongkong8 skykongkong8 changed the title [ unittest ] Fix modelfile TCs @open sesame 11/07 13:26 [ unittest ] Fix modelfile TCs @open sesame 11/07 16:09 Nov 7, 2024
Copy link
Collaborator

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

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

@skykongkong8, 💯 All CI checkers are successfully verified. Thanks.

// #endif
// }
#if defined(ENABLE_NNSTREAMER_BACKBONE) || defined(ENABLE_TFLITE_BACKBONE)
EXPECT_EQ(NN.loadFromConfig(s.getIniName()), ML_ERROR_INVALID_PARAMETER);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems backbone_random_external from ScopdeIni is simply random.tflite; if ENABLE_TFLITE_BACKBONE is defined, shouldn't it expect ML_ERROR_NONE?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are correct, but we are already checking such case with backbone_valid_external from another TC, and I could not find any file for formulating random.tflite currently. I thought it is just a negative TC to emit error like this.

Copy link
Member

@SeoHyungjun SeoHyungjun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@baek2sm baek2sm left a comment

Choose a reason for hiding this comment

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

LGTM

@jijoongmoon jijoongmoon merged commit cbe88ce into nnstreamer:main Nov 8, 2024
49 of 57 checks passed
@skykongkong8 skykongkong8 deleted the pr/modelfile/backbones branch November 10, 2024 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants