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

[onert/api] Add nnfw_train_get_traininfo #12391

Merged
merged 5 commits into from
Jan 3, 2024

Conversation

zetwhite
Copy link
Contributor

@zetwhite zetwhite commented Dec 29, 2023

This PR adds nnfw_train_get_traininfo API.
This api is to get training info from the session.

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

related : #11692

This PR adds nnfw_train_get_traininfo API.
This api is to get training info from the session.

ONE-DCO-1.0-Signed-off-by: SeungHui Youn <sseung.youn@samsung.com>
@zetwhite zetwhite added approval: 2 Require at least 2 approvals PR/ready for review It is ready to review. Please review it. labels Dec 29, 2023
@zetwhite zetwhite requested a review from a team December 29, 2023 07:43
@@ -192,6 +195,7 @@ typedef enum

typedef enum
{
NNFW_TRAIN_OPTIMIZER_UNDEF = -1,
Copy link
Contributor Author

@zetwhite zetwhite Dec 29, 2023

Choose a reason for hiding this comment

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

Q : Why *_UNDEF are added to enums?

A :
nnfw_train_get_traininfo api is to get training info for the user application.
Since we can NOT guarantee that the session always holds every training information,
some fields can be invalid value. (missing value is represented as invalid in core by default)

So, To represent invalid value in the user application, *_UNDEF fields become necessary.

@zetwhite zetwhite removed the PR/ready for review It is ready to review. Please review it. label Dec 29, 2023
@zetwhite zetwhite added the PR/ready for review It is ready to review. Please review it. label Dec 29, 2023
@zetwhite zetwhite requested review from a team and jyoungyun January 2, 2024 02:24
jyoungyun
jyoungyun previously approved these changes Jan 2, 2024
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

@jyoungyun jyoungyun requested a review from a team January 2, 2024 02:56
@@ -176,12 +176,15 @@ NNFW_STATUS nnfw_pop_pipeline_output(nnfw_session *session, void *outputs);
//////////////////////////////////////////////
typedef enum
{
NNFW_TRAIN_LOSS_UNDEF = -1,
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer full name (UNDEFINED) instead of UNDEF.

Suggested change
NNFW_TRAIN_LOSS_UNDEF = -1,
NNFW_TRAIN_LOSS_UNDEFINED = -1,

Also, I like to use 0 for undefined. But I don't want to block this PR. Let's change if others agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upated. PTAL :)

runtime/onert/api/include/nnfw_experimental.h Outdated Show resolved Hide resolved
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

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

@glistening glistening merged commit 094fc01 into Samsung:master Jan 3, 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.

None yet

3 participants