Skip to content

1476 hoeffding races#1842

Open
kulbachcedric wants to merge 4 commits intoonline-ml:1476-hoeffding-racesfrom
kulbachcedric:1476-hoeffding-races
Open

1476 hoeffding races#1842
kulbachcedric wants to merge 4 commits intoonline-ml:1476-hoeffding-racesfrom
kulbachcedric:1476-hoeffding-races

Conversation

@kulbachcedric
Copy link
Copy Markdown
Contributor

Added Hoeffding Race Classifier and Regressor

Copy link
Copy Markdown
Member

@MaxHalford MaxHalford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Approved with some comments. Also, don't forget to add an unreleased.md entry.

I'd love to benchmark this against the other model selection methods.

Comment on lines +42 to +43
models: list[base.Estimator]
metric: metrics.base.Metric
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you confident here? It's not really a pattern we use elsewhere in the codebase, right? I think attributes you declare this way are shared by all instances.

I would be more inclined to simply instantiate a self._models and self._metric in the __init__.

Comment on lines +129 to +132
@property
def n_active_models(self) -> int:
"""The number of models still in the race."""
return len(self._active)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a bit overkill

def predict_proba_one(self, x):
return self.best_model.predict_proba_one(x)

@property
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this doesn't break anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants