-
Notifications
You must be signed in to change notification settings - Fork 16
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
add convergence early stop #16
Conversation
@@ -129,6 +129,7 @@ class OptConfig: | |||
param_save_path: str = field(default=None) | |||
frugal_eval_cost: bool = field(default=True) | |||
use_SH_allocation: bool = field(default=False) | |||
patience: tuple[float,float,int] = field(default=(0.01,0.01,5)) |
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.
add more description to what each element in the tuple means. better with a macro, at least add a comment here and when you use it
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.
added doc string for this class
mscore, mcost = self.get_eval_feedback(eval_result) | ||
if not self._should_stop and self._patience_budget is not None: | ||
impv = False | ||
score_threshold = self.top_down_info.opt_config.patience[0] |
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 change index from absolute value to macros? at least add comment for what the indices 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.
changed to use descriptive macro
cost_threshold = self.top_down_info.opt_config.patience[1] | ||
# reset if score or cost is improved | ||
if mscore is not None and mscore >= score_threshold * (1 + score_threshold): | ||
self._patience_budget = self.top_down_info.opt_config.patience[2] |
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 does patience budget 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.
commented at class attribute now
score_threshold = self.top_down_info.opt_config._early_stop_quality_delta | ||
cost_threshold = self.top_down_info.opt_config._early_stop_cost_delta | ||
# reset if score or cost is improved | ||
if current_score is not None and current_score >= score_threshold * (1 + score_threshold): |
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 is this score_threshold * (1 + score_threshold)
? shouldn't it be comparing to previous score?
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.
now using the best value so far to decide convergence.
if current_score is not None and current_score >= score_threshold * (1 + score_threshold): | ||
self._patience_budget = self.top_down_info.opt_config._early_stop_n_iteration | ||
impv = True | ||
if current_cost is not None and current_cost <= cost_threshold * (1 - cost_threshold): |
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.
same question as score_threshold
No description provided.