-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add a parallel algorithm for TPE and related research blog #1081
Conversation
@@ -190,7 +190,7 @@ class HyperoptTuner(Tuner): | |||
HyperoptTuner is a tuner which using hyperopt algorithm. | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you give me a comparison between this PR and the current method in our example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not clear, compare what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compare with the current TPE version in NNI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PurityFan cannot access PAI anymore, @Crysple will compare the performance.
@@ -374,8 +401,18 @@ def get_suggestion(self, random_search=False): | |||
total_params : dict | |||
parameter suggestion | |||
""" | |||
|
|||
rval = self.rval | |||
if self.parallel == True and len(self.running_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest changing "if self.parallel and len(self.runing_data)" to avoid pylint warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
|
||
rval = self.rval | ||
if self.parallel == True and len(self.running_data): | ||
self.CL_rval = copy.deepcopy(self.rval) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every time we copy a "self.rval" to "self.CL_rval". Shall we need to consider the memory recycle here?
@@ -204,7 +204,12 @@ def __init__(self, algorithm_name, optimize_mode='minimize'): | |||
self.json = None | |||
self.total_data = {} | |||
self.rval = None | |||
self.CL_rval = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the "CL" means? What's the "self.CL_rval" represent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constant Liar
'classArgs': { | ||
Optional('optimize_mode'): setChoice('optimize_mode', 'maximize', 'minimize'), | ||
Optional('parallel_optimize'): setType('parallel_optimize', bool), | ||
Optional('constant_liar_type'): setChoice('min', 'max', 'mean') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is inconsistent with your API in the config file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, the name should also be the first arg of setChoice
closed, this pr is replaced with another pr #1373 |
#1052