Add predict nonlinearity #662
Conversation
Fails in PyTorch 1.1
Fails in PyTorch 1.1 There was one more instance in a unit test that needed fixing.
skorch/net.py
Outdated
| @@ -1010,6 +1039,46 @@ def infer(self, x, **fit_params): | |||
| return self.module_(**x_dict) | |||
| return self.module_(x, **fit_params) | |||
|
|
|||
| def get_predict_nonlinearity(self): | |||
There was a problem hiding this comment.
With this being public there will be two ways to adjust the nonlinearity:
- With the
predict_nonlinearityparameter in init. - Subclassing
Are we okay with this?
There was a problem hiding this comment.
Good question, I was also unsure about this. The predict_nonlinearity parameter specifies what nonlinearity I want, whereas the get_predict_nonlinearity specifies how the nonlinearity selection is resolved. I would also be okay with making the latter private (with the possibility to make it public later if the need ever arises).
There was a problem hiding this comment.
I would go with option 1 for now and having this function be public.
If a user were to override _get_predict_nonlinearity, we would not officially support it and it may break in the future.
There was a problem hiding this comment.
Just to be sure, option 1 for you means making get_predict_nonlinearity private?
Co-authored-by: Thomas J. Fan <[email protected]>
Co-authored-by: Thomas J. Fan <[email protected]>
Co-authored-by: Thomas J. Fan <[email protected]>
Co-authored-by: Thomas J. Fan <[email protected]>
Co-authored-by: Thomas J. Fan <[email protected]>
skorch/net.py
Outdated
| @@ -1010,6 +1039,46 @@ def infer(self, x, **fit_params): | |||
| return self.module_(**x_dict) | |||
| return self.module_(x, **fit_params) | |||
|
|
|||
| def get_predict_nonlinearity(self): | |||
There was a problem hiding this comment.
I would go with option 1 for now and having this function be public.
If a user were to override _get_predict_nonlinearity, we would not officially support it and it may break in the future.
skorch/tests/test_classifier.py
Outdated
| @@ -289,7 +289,12 @@ def test_custom_loss_does_not_call_sigmoid( | |||
| mock = Mock(side_effect=lambda x: x) | |||
| monkeypatch.setattr(torch, "sigmoid", mock) | |||
|
|
|||
| net = net_cls(module_cls, max_epochs=1, lr=0.1, criterion=nn.MSELoss) | |||
| # we need to add a custom nonlinearity, otherwise the output won't be 2d | |||
There was a problem hiding this comment.
this comment does not help me understand as it does not answer why the output won't be in 2d
| # don't want callbacks to trigger side effects | ||
| net.callbacks_ = [] | ||
| net.partial_fit(X, y) | ||
| assert not side_effect |
There was a problem hiding this comment.
But wouldn't we expect callbacks such as accuracy scoring to call predict?
There was a problem hiding this comment.
That's why I removed all callbacks two lines earlier, otherwise the test becomes very messy.
skorch/tests/test_net.py
Outdated
| def test_predict_nonlinearity_none( | ||
| self, net_cls, module_cls, data): | ||
| # even though we have CrossEntropyLoss, we don't want the | ||
| # output from predict_proba to be modified, since we set |
There was a problem hiding this comment.
| # output from predict_proba to be modified, since we set | |
| # output from predict_proba to be modified, thus we set |
|
|
||
| def test_infer_neural_binary_net_classifier_default( | ||
| self, infer_predict_nonlinearity, net_bin_clf_cls, module_cls): | ||
| # BCEWithLogitsLoss should return valid probabilities |
There was a problem hiding this comment.
| # BCEWithLogitsLoss should return valid probabilities | |
| # BCEWithLogitsLoss criterion: nonlinearity should return valid probabilities |
skorch/utils.py
Outdated
|
|
||
| Sigmoid is applied to x to transform it to probabilities. Then | ||
| concatenate the probabilities with 1 - these probabilities to | ||
| return a correctly formed ``y_proba``. |
There was a problem hiding this comment.
Maybe specify who expects this format. Something along the lines of formats outputs for use with BCE loss or something like that
Co-authored-by: ottonemo <[email protected]>
Improve explanations in comments and docstrings.
|
@ottonemo I addressed your comments. |
Resolves #637, resolves #661
Supersedes #572 #580
Added a parameter
predict_nonlinearitytoNeuralNetwhich allows users to control the nonlinearity to be applied to the module output when callingpredictandpredict_proba. Also, when usingCrossEntropyLoss, softmax is now automatically applied to the output.This PR implements what was discussed in #661. Regarding the implementation, I used the most simple approach at the moment (no dispatching based on the net class, for instance). Also, I added a
get_predict_nonlinearitymethod toNeuralNet, which handles the case of'auto'vsNonevscallable.Regarding the naming, I decided against
output_nonlinearity. I believepredict_nonlinearitymakes it much more obvious that this nonlinearity is not applied to the module output directly (which would affect loss etc.) but only when callingpredictandpredict_proba.ping @qtux