Add new learners (mostly regressors) #377
Conversation
- Add imports, default parameter grids, and some keyword argument fixing.
- Also update fixed parameters and param search grids to include already missing learners.
- This way it's easier to tell which specific sub-test failed.
- There are too many parameters and it slows down grid search too much to specify all possible combinations.
skll/learner.py
Outdated
| KNeighborsClassifier: | ||
| [{'n_neighbors': [1, 5, 10, 100], | ||
| 'weights': ['uniform', 'distance']}], | ||
| KNeighborsRegressor: | ||
| [{'n_neighbors': [1, 5, 10, 100], | ||
| 'weights': ['uniform', 'distance']}], | ||
| MLPClassifier: | ||
| [{}], |
There was a problem hiding this comment.
I know searching over hidden_layer_sizes takes too long to search over by default, but what about the other parameters like activation, solver, learning_rate, etc?
There was a problem hiding this comment.
I didn't include the other parameters either because:
(a) Many of the hyper-parameters are conditionally dependent on each other. For example, learning_rate is only used when solver = sgd, learning_rate_init is only used when solver = sgd or adam, and early_stopping is only used when solver = sgd. So, it's very different from other learners in that sense and there isn't an easy way to put together a product over all the parameters in the grid that will all be valid together.
(b) Many of the hyper-parameters shouldn't just be blindly iterated over. For example, the lbfgs solver is not great when you have a lot of data but when you have a small amount of data, it's better but still quite slow. Whether or not to use early stopping interacts with whether you have sufficient data in the first place.
IMO, these decisions about which sets of hyper-parameters should be searched together depends on the experiment and the data and are best left to the user. However, if you have a suggestion for a default parameter grid that makes sense (without a lot of extra logic to deal with conditional dependence) then I am happy to try it out.
There was a problem hiding this comment.
Hmm, an option could be to have sgd solver and a single hidden layer be the default for any MLP and then have a grid iterate over activation, learning_rate, and learning_rate_init. In that case, if you want to do something different, you will have to change the solver via fixed_parameters and then also specify your own set of parameters to search via param_grids. Thoughts? @Lguyogiro @aoifecahill @mulhod ?
tests/test_classification.py
Outdated
|
|
||
| # train an AdaBoostRegressor on the training data and evalute on the | ||
| # testing data | ||
| learner = Learner('MLPClassifier', model_kwargs={'solver': 'lbfgs'}) |
There was a problem hiding this comment.
Should this comment say "train an MLPClassifier on the training data and evaluate on the testing data" ?
- And also some default fixed parameters that seem a little more reasonable.
- Also filter out convergence warnings since we don't want those to clutter the test logs.
|
@aoifecahill @mulhod this is now ready for review. @Lguyogiro you can probably take another look since I added the MLP grid this time around. |
| from sklearn.linear_model.base import LinearModel | ||
| from sklearn.metrics import (accuracy_score, | ||
| confusion_matrix, | ||
| precision_recall_fscore_support, | ||
| SCORERS) |
There was a problem hiding this comment.
do we not need this any more?
There was a problem hiding this comment.
It was imported but never used in that file so I removed it.
There was a problem hiding this comment.
Ah, makes sense, thanks!
Add the following new regressors:
BayesianRidgeDummyRegressorHuberRegressorsLarsMLPRegressorRANSACRegressorTheilSenRegressorAdd the following new classifiers:
MLPClassifierRidgeClassifierAdd default parameter grids where appropriate for these new learners. Blank default parameter grids for learners that do not have any hyper-parameters that make sense to tune by default and the decision is better left to the user.
Update documentation.
Update tests.