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

[tools/onert_train] Get training info from session #12411

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

zetwhite
Copy link
Contributor

@zetwhite zetwhite commented Jan 5, 2024

This PR adds logic of getting training information from the session in onert_train.

ONE-DCO-1.0-Signed-off-by: SeungHui Youn sseung.youn@samsung.com

related issue : #11692

@zetwhite zetwhite added approval: 2 Require at least 2 approvals PR/ready for review It is ready to review. Please review it. labels Jan 5, 2024
@zetwhite zetwhite requested a review from a team January 5, 2024 10:59
Copy link
Contributor

@YongseopKim YongseopKim left a comment

Choose a reason for hiding this comment

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

LGTM and I have another question.

[onert/test] Get training info from session in onert_train

The PR title's prefix is onert/test. I thought it could be onert/onert_train. Is any rule to have a consensus?

@zetwhite
Copy link
Contributor Author

zetwhite commented Jan 8, 2024

The PR title's prefix is onert/test. I thought it could be onert/onert_train. Is any rule to have a consensus?

Umm. It looks like there are no strict rules. The most commonly used prefix is oner/test and tools/onert_train.

Thanks for catching out. I renamed PR title a bit, to emphasize this PR is related to onert_train.

➜ git log --oneline | grep '\[onert\/test\]*'
b2c664f16 [onert/test] Add util to print nnfw_train_info (#12296)
f3a10c40d [onert/test] Merge test function (#11512)
efb8c1894 [onert/test] Add quantization option in onert_run (#11473)
4b415f525 [onert/test] Fix meaningless test (#10614)
02166b58c [onert/test] Revise run_test.sh (#10529)
... 
97 results 
➜ git log --oneline | grep '\[tools\/onert_train\]*'        
716bb3d13 [tools/onert_train] Add vdata_length check logic (#12379)
be0f0b889 [tools/onert_train] Use metric parameter (#12351)
758cae579 [tools/onert_train] Apply validation split value (#12339)
7f1db5e93 [tools/onert_train] Add the required software tools to readme file (#12334)
8533c5a00 [tools/onert_train] Introduce DataLoader class (#12324)
... 
31 results 

@zetwhite zetwhite changed the title [onert/test] Get training info from session in onert_train [tools/onert_train] Get training info from session Jan 8, 2024
This PR adds logic of getting training information from the session in onert_train.

ONE-DCO-1.0-Signed-off-by: SeungHui Youn <sseung.youn@samsung.com>
Copy link
Contributor

@glistening glistening left a comment

Choose a reason for hiding this comment

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

LGTM

@jyoungyun
Copy link
Contributor

jyoungyun commented Jan 8, 2024

@YongseopKim

The PR title's prefix is onert/test. I thought it could be onert/onert_train. Is any rule to have a consensus?

AFAIK, there is no detailed rule about this. I usually use tools/onert_train because it is not enough to explain what the target is when using tools/test.

Copy link
Contributor

@jyoungyun jyoungyun left a comment

Choose a reason for hiding this comment

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

LGTM

@glistening glistening merged commit 8f74b44 into Samsung:master Jan 8, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: 2 Require at least 2 approvals PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants