-
Notifications
You must be signed in to change notification settings - Fork 1.8k
support trial config deduplication in SMAC tuner #1840
Conversation
self.total_data[parameter_id] = challenger | ||
return self.param_postprocess(challenger.get_dictionary()) | ||
assert self.dedup is True |
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.
I think we don't need the assert here
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.
i want it to have a quick fail
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.
why the self.dedup
must be True
? Seems it all depends on the parameter config_dedup
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 if challengers
is empty?
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.
assert challengers not empty before the for loop will make it more straightforward
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.
https://github.com/automl/SMAC3/blob/81eba541e67c09aa307f2ba23d583f94c60b6125/smac/optimizer/ei_optimization.py#L699 this is the implementation of challengers, I am not sure whether it is good to use len(challengers.challengers)
, seems very ugly.
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.
Got it, I agree that not to use challengers.chanllengers since it is intenal implementation. I am OK with current code, or add another boolean variable to record and check whether challengers is empty.
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.
updated, thanks.
when
config_dedup
is True, a configuration will not be generated more than once