Several improvements to NeuralNetBinaryClassifier #515
Conversation
* more fit runs because of flaky output (fixes #514) * add a classes_ attribute for things like CalibratedClassifierCV (fixes #465) * make y_proba 2-dim, which works better with certain sklearn metrics (fixes #442) * automatically squeezes output if module returns (n,1) shape (fixes #502) This could break backwards compatibility if someone relies on y_proba to be 1d.
|
@ZaydH Since you worked and had trouble with |
skorch/tests/test_classifier.py
Outdated
| @@ -196,10 +196,10 @@ def test_predict_predict_proba(self, net, data, threshold): | |||
| net.fit(X, y) | |||
|
|
|||
| y_pred_proba = net.predict_proba(X) | |||
| assert y_pred_proba.ndim == 1 | |||
| assert y_pred_proba.shape[1] == 2 | |||
There was a problem hiding this comment.
Nit
| assert y_pred_proba.shape[1] == 2 | |
| assert y_pred_proba.shape == (X.shape[0], 2) |
skorch/classifier.py
Outdated
| # pylint: disable=signature-differs | ||
| def check_data(self, X, y): | ||
| super().check_data(X, y) | ||
| if get_dim(y) != 1: | ||
| raise ValueError("The target data should be 1-dimensional.") | ||
|
|
||
| def infer(self, x, **fit_params): | ||
| y_infer = super().infer(x, **fit_params) | ||
| if y_infer.dim() < 2: |
There was a problem hiding this comment.
If we want to be really direct:
| if y_infer.dim() < 2: | |
| if y_infer.dim() == 1: |
CHANGES.md
Outdated
|
|
||
| ### Changed | ||
|
|
||
| - Improve numerical stability when using `NLLLoss` in `NeuralNetClassifer` (#491) | ||
| - NeuralNetBinaryClassifier.predict_proba now returns a 2-dim array; to access the "old" y_proba, take y_proba[:, 1] |
skorch/tests/test_classifier.py
Outdated
|
|
||
| def test_with_calibrated_classifier_cv(self, net_fit, data): | ||
| from sklearn.calibration import CalibratedClassifierCV | ||
| cccv = CalibratedClassifierCV(net_fit, cv=3) |
There was a problem hiding this comment.
Do we need the fitted module here?
If we care alot about test run time speed, we can set cv=2?
There was a problem hiding this comment.
Changed to cv=2. Net has to be initialized for this to work.
skorch/tests/test_classifier.py
Outdated
| @@ -166,7 +166,7 @@ def test_not_fitted_raises(self, net_cls, module_cls, data, method): | |||
| "before using this method.") | |||
| assert exc.value.args[0] == msg | |||
|
|
|||
| @flaky(max_runs=3) | |||
| @flaky(max_runs=5) | |||
There was a problem hiding this comment.
That is getting pretty flaky indeed.
There was a problem hiding this comment.
I made a grid search (yay) to find better parameters. Hopefully the test is less flaky now.
|
@thomasjpfan Pls review again |
| # pylint: disable=signature-differs | ||
| def check_data(self, X, y): | ||
| super().check_data(X, y) | ||
| if get_dim(y) != 1: | ||
| raise ValueError("The target data should be 1-dimensional.") | ||
|
|
||
| def infer(self, x, **fit_params): |
There was a problem hiding this comment.
I think a docstring that documents the automatic squeezing of module outputs would be helpful here (i.e., that it is done and why it is done).
Co-Authored-By: ottonemo <[email protected]>
* add a docstring * correct error message when output dim > 2 * also works when more than 1 output is returned
…ithub.com/skorch-dev/skorch into feature/improvements-to-binary-classifier
|
@ottonemo I changed the requested things, and then some more minor things. Please review again. |
This could break backwards compatibility if someone relies on y_proba to be 1d.