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.

Sign up for GitHub

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

Jump to bottom

[Unittest] FSU Unittest with Simple FC Model #2835

Merged
merged 1 commit into from
Jan 2, 2025
Merged

[Unittest] FSU Unittest with Simple FC Model #2835

merged 1 commit into from
Jan 2, 2025

Conversation

DonghakPark
Copy link
Member

Add Unittest for FSU

  • Use Simple FC Model that consist with 6-FC layer
  • FP16-FP16 / Lookahead 1

i will add more case of FSU Model at Benchmark

Self evaluation:

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

EunjuYang
EunjuYang reviewed Dec 23, 2024
View reviewed changes
Copy link
Contributor

@EunjuYang EunjuYang left a comment

Choose a reason for hiding this comment

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

Overall LGTM :) I have some questions on this PR. Could you answer to this? Thank you.

test/unittest/integration_tests/integration_test_fsu.cpp Outdated
Comment on lines 62 to 81
model->addLayer(ml::train::createLayer(
"fully_connected",
{withKey("unit", 1000), withKey("weight_initializer", "xavier_uniform"),
withKey("bias_initializer", "zeros")}));
model->addLayer(ml::train::createLayer(
"fully_connected",
{withKey("unit", 1000), withKey("weight_initializer", "xavier_uniform"),
withKey("bias_initializer", "zeros")}));
model->addLayer(ml::train::createLayer(
"fully_connected",
{withKey("unit", 1000), withKey("weight_initializer", "xavier_uniform"),
withKey("bias_initializer", "zeros")}));
model->addLayer(ml::train::createLayer(
"fully_connected",
{withKey("unit", 1000), withKey("weight_initializer", "xavier_uniform"),
withKey("bias_initializer", "zeros")}));
model->addLayer(ml::train::createLayer(
"fully_connected",
{withKey("unit", 1000), withKey("weight_initializer", "xavier_uniform"),
withKey("bias_initializer", "zeros")}));
Copy link
Contributor

Choose a reason for hiding this comment

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

What about updating this duplicate code blocks with loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea i will fix that !

test/unittest/integration_tests/integration_test_fsu.cpp
Comment on lines +101 to +89
model->save("simplefc_weight_fp16_fp16_100.bin",
ml::train::ModelFormat::MODEL_FORMAT_BIN);
model->load("./simplefc_weight_fp16_fp16_100.bin");
Copy link
Contributor

Choose a reason for hiding this comment

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

Simple question. Why do we need this save and load code? Is it because it only supports inference mode now?

Copy link
Member Author

Choose a reason for hiding this comment

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

for test FSU, we need file that stored in storage. so save model file, and load files for now
--> in real Inference case, they have bin files already

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yes, that's what the FSU is !🤣 Thank you for the kind reply :)

test/unittest/integration_tests/integration_test_fsu.cpp
in.push_back(input);
l.push_back(label);

answer = model->inference(1, in, l);
Copy link
Contributor

Choose a reason for hiding this comment

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

How can we check the FSU works from this test code?

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 can check mem_usage.
actually i add this case for FSU build test, not for detail works
--> working test will be add to benchmarks tool

@DonghakPark
[Unittest] FSU Unittest with Simple FC Model
9abf939 
Add Unittest for FSU
- Use Simple FC Model that consist with 6-FC layer
- FP16-FP16 / Lookahead 1

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

Signed-off-by: Donghak PARK <[email protected]>
@myungjoo
Copy link
Member

LGTM.

Question on FSU in general:

  • Would it be possible to use FSU for inference without calling any C++ APIs except for the simple inference APIs?
    e.g., (pseudo code)
h = open_model(modelfile_path);
h.configure(FSU=true);
output = h.run(input);
h = close_model();

@DonghakPark
Copy link
Member Author

LGTM.

Question on FSU in general:

  • Would it be possible to use FSU for inference without calling any C++ APIs except for the simple inference APIs?
    e.g., (pseudo code)
h = open_model(modelfile_path);
h.configure(FSU=true);
output = h.run(input);
h = close_model();

i think It is possible to activate FSU by simply setting Setproperty after loading the model that train has been completed.

skykongkong8
skykongkong8 approved these changes Dec 24, 2024
View reviewed changes
Copy link
Member

@skykongkong8 skykongkong8 left a comment

Choose a reason for hiding this comment

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

LGTM!

@myungjoo
Copy link
Member

LGTM.
Question on FSU in general:

  • Would it be possible to use FSU for inference without calling any C++ APIs except for the simple inference APIs?
    e.g., (pseudo code)
h = open_model(modelfile_path);
h.configure(FSU=true);
output = h.run(input);
h = close_model();

i think It is possible to activate FSU by simply setting Setproperty after loading the model that train has been completed.

In the end, an app like the "f" should be able to do so.

@DonghakPark
Copy link
Member Author

LGTM.
Question on FSU in general:

  • Would it be possible to use FSU for inference without calling any C++ APIs except for the simple inference APIs?
    e.g., (pseudo code)
h = open_model(modelfile_path);
h.configure(FSU=true);
output = h.run(input);
h = close_model();

i think It is possible to activate FSU by simply setting Setproperty after loading the model that train has been completed.

In the end, an app like the "f" should be able to do so.

That's a great idea. By raising the level of abstraction, allowing users with pre-trained weights to simply activate the swap functionality using something like model.Run(parm) would significantly enhance convenience during development.
for this i will make issue

@DonghakPark DonghakPark mentioned this pull request Dec 25, 2024
Open
djeong20
djeong20 approved these changes Dec 30, 2024
View reviewed changes
Copy link
Contributor

@djeong20 djeong20 left a comment

Choose a reason for hiding this comment

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

Nice work!

jijoongmoon
jijoongmoon approved these changes Jan 2, 2025
View reviewed changes
Copy link
Collaborator

@jijoongmoon jijoongmoon 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 8184b61 into nnstreamer:main Jan 2, 2025
23 checks passed
@DonghakPark DonghakPark deleted the fsu_unittest branch January 13, 2025 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skykongkong8 skykongkong8 skykongkong8 approved these changes

@djeong20 djeong20 djeong20 approved these changes

@jijoongmoon jijoongmoon jijoongmoon approved these changes

@EunjuYang EunjuYang EunjuYang approved these changes

@myungjoo myungjoo myungjoo approved these changes

@again4you again4you Awaiting requested review from again4you again4you is a code owner

@jaeyun-jung jaeyun-jung Awaiting requested review from jaeyun-jung jaeyun-jung is a code owner

@leemgs leemgs Awaiting requested review from leemgs leemgs is a code owner

@wooksong wooksong Awaiting requested review from wooksong wooksong is a code owner

@gichan-jang gichan-jang Awaiting requested review from gichan-jang gichan-jang is a code owner

@anyj0527 anyj0527 Awaiting requested review from anyj0527 anyj0527 is a code owner

@lhs8928 lhs8928 Awaiting requested review from lhs8928 lhs8928 is a code owner

@songgot songgot Awaiting requested review from songgot songgot is a code owner

@jihochu jihochu Awaiting requested review from jihochu jihochu is a code owner

@SeoHyungjun SeoHyungjun Awaiting requested review from SeoHyungjun SeoHyungjun is a code owner

@baek2sm baek2sm Awaiting requested review from baek2sm baek2sm is a code owner

Assignees
No one assigned
Labels
PR/READY2MERGE
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@DonghakPark @myungjoo @EunjuYang @jijoongmoon @djeong20 @skykongkong8