added unit testing for ContextualizedClassifier models#153
Conversation
cnellington
left a comment
cnellington
left a comment
There was a problem hiding this comment.
LGTM! One comment to address.
| alpha=1e-1, encoder_type='mlp' | ||
| ) | ||
| self._quicktest( | ||
| model, C, X, Y, max_epochs=10 |
There was a problem hiding this comment.
Has 10 epochs been enough in your experience to get NaN values from the old classifier?
There was a problem hiding this comment.
Using the sciPlex2 dataset, I used to get NaN values within the first epoch, so I assume 10 epochs will be enough for this test case.
Also, in my local machine, there are times when training in test_classifier.py and test_regressor.py ends before it reaches the assigned number of epochs. I am not sure why it happens, but could you confirm if it also happens in your machine?
There was a problem hiding this comment.
Early stopping is enabled by default for the easy modules, but we should turn this off when testing the regression and classifier modules. Here's the relevant kwargs/callback constructor: https://github.com/cnellington/Contextualized/blob/30cb09c0261899584c61eb93997f1ca8a624ca3e/contextualized/easy/wrappers/SKLearnWrapper.py#L167
You should be able to disable this by setting es_patience to something like -1 or inf.
| self._quicktest( | ||
| model, C, X, Y, max_epochs=10, es_patience=float('inf') | ||
| ) | ||
|
|
There was a problem hiding this comment.
Lots of whitespace, let's run black and make a new pylint badge for this update. Tests look great though.
2 participants
This is a follow-up on ContextualizedClassifier with BCELoss #140
test_classifier checks:
Note: ran black on contextualized and made a new pylint badge