check if model is paralizable#230
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #230 +/- ##
===========================================
+ Coverage 88.16% 88.18% +0.01%
===========================================
Files 23 23
Lines 1884 1904 +20
===========================================
+ Hits 1661 1679 +18
- Misses 223 225 +2
Continue to review full report at Codecov.
|
|
|
||
| return ret | ||
|
|
||
| def model_single_core(model): |
There was a problem hiding this comment.
Maybe call this _check_n_jobs?
| print('Warning! Using subclass BaseSearchCV other than ' \ | ||
| '{GridSearchCV, RandomizedSearchCV}. Should implement param check. ') | ||
| pass | ||
|
|
| isinstance(model, sklearn.model_selection._search.BaseSearchCV)): | ||
| raise ValueError('model should be BaseEstimator or BaseSearchCV') | ||
|
|
||
| # check if the njobs is not in the optimization trace |
There was a problem hiding this comment.
I think you want to say: make sure that n_jobs is not in the parameter grid.
| 'optimize the n_jobs parameter.') | ||
|
|
||
| # check the parameters for n_jobs | ||
| if check(model.get_params(), False) == False: |
There was a problem hiding this comment.
You coud do return check(model.get_params(), False)
|
|
||
| __all__ = ['OpenMLFlow', 'create_flow_from_model', 'get_flow', 'list_flows', | ||
| 'sklearn_to_flow', 'flow_to_sklearn', 'flow_exists'] | ||
| 'sklearn_to_flow', 'flow_to_sklearn', 'flow_exists', 'model_is_paralizable'] |
There was a problem hiding this comment.
What is model_is_paralizable?
| sklearn.ensemble.RandomForestClassifier(n_jobs=5), | ||
| sklearn.ensemble.RandomForestClassifier(n_jobs=-1), | ||
| sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=1))]), | ||
| sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=5))]), |
There was a problem hiding this comment.
Shouldn't that fail because n_jobs > 1?
| sklearn.ensemble.RandomForestClassifier(n_jobs=-1), | ||
| sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=1))]), | ||
| sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=5))]), | ||
| sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=-1))]), |
There was a problem hiding this comment.
Shouldn't that fail because n_jobs < 0?
| legal_models = [ | ||
| sklearn.ensemble.RandomForestClassifier(), | ||
| sklearn.ensemble.RandomForestClassifier(n_jobs=5), | ||
| sklearn.ensemble.RandomForestClassifier(n_jobs=-1), |
There was a problem hiding this comment.
Shouldn't that fail because n_jobs < 0?
|
|
||
| legal_models = [ | ||
| sklearn.ensemble.RandomForestClassifier(), | ||
| sklearn.ensemble.RandomForestClassifier(n_jobs=5), |
There was a problem hiding this comment.
Shouldn't that fail because n_jobs > 1?
There was a problem hiding this comment.
Yes, this is exactly what happens, and this is specified in the answers array. Legal means here 'will not raise an error'. i added comments to make this more clear
| sklearn.model_selection.GridSearchCV(multicore_bagging, illegal_param_dist) | ||
| ] | ||
|
|
||
| answers = [True, False, False, True, False, False, True, False] |
There was a problem hiding this comment.
Ah, here it's obvious what you're doing. Maybe you can re-order the test to make it easier to comprehend (i.e. check the legal models before declaring the illegal models, and maybe giving the list a different name.
There was a problem hiding this comment.
I added some comments
3 participants
No description provided.