[Wait for #2849] [ Context ] Add Engine Class to manage Contexts #2848
Conversation
This PR add ComputeEngine Enum Property. Enum elements are "cpu", "gpu", "qnn" for now. The property format is "engine=qnn". **Self evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: jijoong.moon <[email protected]>
jijoongmoon
requested review from
myungjoo,
again4you,
jaeyun-jung,
leemgs,
wooksong,
gichan-jang,
anyj0527,
lhs8928,
songgot,
jihochu,
DonghakPark,
SeoHyungjun,
baek2sm,
skykongkong8,
djeong20 and
EunjuYang
as code owners
jijoongmoon
force-pushed
the
engine_class
branch
from
996fa4f to
af1b279
Compare
jijoongmoon
changed the title
jijoongmoon
force-pushed
the
engine_class
branch
2 times, most recently
from
9d38a3a to
a2e6707
Compare
| createLayerObject(const std::string &type, | ||
| const std::vector<std::string> &properties = {}) const { | ||
| auto ct = getRegisteredContext(parseComputeEngine(properties)); | ||
| ct->getName(); |
There was a problem hiding this comment.
This line (L132) seems redundant. The return value is not used.
| @@ -37,6 +37,7 @@ | |||
| #include <common_properties.h> | |||
| #include <compiler_fwd.h> | |||
| #include <dynamic_training_optimization.h> | |||
| #include <engine.h> | |||
There was a problem hiding this comment.
Please remove #include <app_contxt.h> as well.
nntrainer/nntrainer/models/neuralnet.h
Line 36 in 24a868d
| if (!(options & LayerCreateSetPropertyOptions::AVAILABLE_FROM_APP_CONTEXT)) { | ||
| ac.registerFactory<nntrainer::Layer>(std::get<0>(GetParam())); | ||
| ac->registerFactory<nntrainer::Layer>(std::get<0>(GetParam())); |
There was a problem hiding this comment.
I have a question.
In this PR, createOptimizerObject is implemented as an interface for engine, but registerFactory is not.
I guess registerFactory can also be an another common function used in various contexts; in this sense, it might be better to be implemented in Context class and called at the Engine according to its property. By doing so, only the engine header will be explosed to the outside. Could you share your idea on this? I couldn't catch what is the standard allowing to access via engine.
There was a problem hiding this comment.
yes. it should be. this should be done in later PR. I will leave the todo comments.
jijoongmoon
force-pushed
the
engine_class
branch
2 times, most recently
from
25d5cc9 to
dbbde82
Compare
In order to extend the Context more easily, this PR adds Engine Class to manage the Contexts. It adds Context Class as well which is base class of all the Context. . add Engine Class . add Context Class . set default Context is app contexts . Pluggable support in Engine . some more code optimization & test codes requires. **Self evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: jijoong.moon <[email protected]>
jijoongmoon
force-pushed
the
engine_class
branch
from
dbbde82 to
89a624e
Compare
Add Engine and Context Class
In order to extend the Context more easily, this PR adds Engine Class
to manage the Contexts. It adds Context Class as well which is base
class of all the Context.
. add Engine Class
. add Context Class
. set default Context is app contexts
. Pluggable support in Engine
. some more code optimization & test codes requires.
Self evaluation:
Signed-off-by: jijoong.moon [email protected]