Grid search classes should accept estimator instances instead of just classes
Scikit learn does, surprise doesn't, and that sucks :)
Current workaround for changing default parameter values that you want to keep constant across searches, e.g. with random_state:
grid = {'random_state': [one_unique_element_in_the_list], 'other_param': [a, b, c]...}
I'd like to take a stab at this. One proposed solution:
- Ensure that all classes save arguments to the
__init__method as self.arg_name - When generating grid searches create model objects which have the same arguments as some initial class overridden by the values for the parameter search (only if whats passed in is an instance of
AlgoBaseelse just do normal behavior).
Awesome! :)
For this I think we should try to follow as close as possible what scikit learn is doing (see e.g. their conventions for estimators). There's no need to reinvent the wheel here and your proposal is going in the right direction.
Ensure that all classes save arguments to the init method as self.arg_name
Yes! We should also ensure that none of those parameters/attributes are modified in __init__ or in any other method. Else, we might end up with different clones.
When generating grid searches create model objects which have the same arguments as some initial class overridden by the values for the parameter search
Yes, that's basically the clone method from scikit learn. This would require a get_params method I believe.