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 TrainingInfo into nnfw_session #12342

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

zetwhite
Copy link
Contributor

@zetwhite zetwhite commented Dec 20, 2023

This PR adds TrainingInfo into nnfw_session.
It also initalize nnfw_session._train_info in 'load_model_from_modelfile' and 'load_model_from_nnpackage'.

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

issue : #11692
draft : #12152.

@zetwhite zetwhite force-pushed the 1220/ir_tinfo_usage branch 3 times, most recently from 1708de6 to 754b1e7 Compare December 21, 2023 11:33
@zetwhite zetwhite marked this pull request as ready for review December 21, 2023 11:45
@zetwhite zetwhite changed the title Draft : [onert/api] Add TrainingInfo into nnfw_session [onert/api] Add TrainingInfo into nnfw_session Dec 21, 2023
This PR adds TrainingInfo into nnfw_session.
It also initalize nnfw_session._train_info in 'load_model_from_modelfile' and 'load_model_from_nnpackage'.

ONE-DCO-1.0-Signed-off-by: SeungHui Youn <sseung.youn@samsung.com>
@@ -206,6 +210,7 @@ struct nnfw_session
std::shared_ptr<onert::api::CustomKernelRegistry> _kernel_registry;
std::vector<std::thread> _threads;
uint32_t _training_step{0};
std::unique_ptr<onert::ir::train::TrainingInfo> _train_info;
Copy link
Contributor Author

@zetwhite zetwhite Dec 21, 2023

Choose a reason for hiding this comment

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

@Samsung/one_onert

Q: Is there a plan to support multiple models in training features?
If so, Since the session hold multiple models through _nnpkg, It might be better to add a vector of Training info

Suggested change
std::unique_ptr<onert::ir::train::TrainingInfo> _train_info;
std::vector<std::unique_ptr<onert::ir::train::TrainingInfo>> _train_info;

I thought there no plan for support multiple model training at near future.
So, I'd like to save first model(primary model)'s TrainingInfo in the session, for now.

@zetwhite zetwhite added the PR/ready for review It is ready to review. Please review it. label Dec 21, 2023
@@ -402,6 +405,9 @@ NNFW_STATUS nnfw_session::load_model_from_nnpackage(const char *package_dir)
_coptions.push_back(onert::compiler::CompilerOptions::fromGlobalConfig());
}

// TODO load TrainingInfo from model, using TraininfoLoader
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(note)
more exaclty load TrainingInfo from "primary model" using TrainInfoLoader

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

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

@zetwhite zetwhite requested a review from a team December 26, 2023 07:31
@hseok-oh hseok-oh merged commit 6145291 into Samsung:master Dec 26, 2023
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