Hoeffding races implementation #1656
Conversation
comments
e10e3
left a comment
e10e3
left a comment
There was a problem hiding this comment.
Thank you for this PR!
Sorry for the delay before the review.
Your proposition looks good, I have a few questions to better understand the code.
Where does this implementation come from? Is it based on a paper, or maybe on an existing piece of code?
If this is an original work, can you explain its benefits?
If this is based on an existing work, please cite your sources.
There was a problem hiding this comment.
Where does this file come from? Is necessary to build River?
| Computes the hoeffding bound according to n, the number of iterations done. | ||
|
|
||
| """ | ||
| return math.sqrt((math.log(1 / self.delta)) / (2 * n)) |
There was a problem hiding this comment.
The usual formula for the Hoeffding bound includes the numeric range of the variable (sometimes expressed as
| return len(self.remaining_models) == 1 | ||
|
|
||
|
|
||
| class HoeffdingRaceRegressor(base.Regressor): |
There was a problem hiding this comment.
The regressor class is almost identical to the classifier. Have you considered factoring the common parts? (You don't have to do it, it's a suggestion)
|
|
||
| def predict_one(self, x): | ||
| if len(self.remaining_models) == 1: | ||
| return self.models[list(self.remaining_models)[0]].predict_one(x) |
There was a problem hiding this comment.
Isn't self.remaining_models already a list? If so, you don't need to convert it to a list again.
| def predict_one(self, x): | ||
| if len(self.remaining_models) == 1: | ||
| return self.models[list(self.remaining_models)[0]].predict_one(x) | ||
| return None |
There was a problem hiding this comment.
I see that your model selector will not make any prediction while it has not converged to one model. Is there a reason to do so?
Could the best model be used for prediction?
What happens is the model selector never converges, i.e. two or three models are always equivalent?
| self.n = 0 | ||
| self.model_metrics = {name: metric.clone() for name in models.keys()} | ||
| self.model_performance = {name: 0 for name in models.keys()} | ||
| self.remaining_models = [i for i in models.keys()] |
There was a problem hiding this comment.
| self.remaining_models = [i for i in models.keys()] | |
| self.remaining_models = list(models.keys()) |
Would this list conversion be more readable/explicit?
| return math.sqrt((math.log(1 / self.delta)) / (2 * n)) | ||
|
|
||
| def learn_one(self, x, y): | ||
| best_perf = max(self.model_performance.values()) if self.n > 0 else 0 |
There was a problem hiding this comment.
Regression metrics are better with decreasing values (we want to minimize the error). For regression, wouldn't one want to select the model with the smallest error as the best model?
| best_perf = max(self.model_performance.values()) if self.n > 0 else 0 | |
| best_perf = min(self.model_performance.values()) if self.n > 0 else math.inf |
| self.model_metrics = {name: metric.clone() for name in models.keys()} | ||
| self.model_performance = {name: 0 for name in models.keys()} |
There was a problem hiding this comment.
What is the difference between self.model_metrics and self.model_performance? It looks to me like model_performance holds the inner value of the metrics and nothing else.
| """ | ||
| HoeffdingRace-based model selection for Classification. | ||
|
|
||
| Each models is associated to a performance (here its accuracy). When the model is considered too inaccurate by the hoeffding bound, |
There was a problem hiding this comment.
| Each models is associated to a performance (here its accuracy). When the model is considered too inaccurate by the hoeffding bound, | |
| Each model is associated to a performance (here its accuracy). When the model is considered too inaccurate by the Hoeffding bound, |
| """ | ||
| HoeffdingRace-based model selection for regression. | ||
|
|
||
| Each models is associated to a performance (here its accuracy). When the model is considered too inaccurate by the hoeffding bound, |
There was a problem hiding this comment.
The metric for regression is an error, the term "accuracy" is only used for classification
| Each models is associated to a performance (here its accuracy). When the model is considered too inaccurate by the hoeffding bound, | |
| Each model is associated to a performance (here its error). When the model is considered too inaccurate by the Hoeffding bound, |
4 participants
No description provided.