Closed gardberg closed 2 years ago
Thanks @LukasGardberg, dont worry about the CI problem. I will leave the detailed comment as soon as possible.
Yes, a bit, but never with pytest. I'm thinking I'll just imitate the tests that already exists. Is there anything specific I should test for?
Also, when running the tests locally I seem to be getting an error:
AttributeError: 'NeuralForestClassifier' object has no attribute 'voting_strategy'
I tried adding an init method to the NeuralForestClassifier
class, but am a bit unsure how to deal with having two superclasses, and how to properly handle this problem. Do you have any advice for how to fix it :)? Sorry for needing some guidance haha
Thanks for your great work @LukasGardberg ! Kind of busy these days, and I will take a look in a few days.
Meanwhile, please click the Details
button of the CI torchensemble-CI / build (ubuntu-latest, 3.7) (pull_request)
and see pytest results, since the neural forest class inherits from voting, its __init__
method should be modified accordingly.
@all-contributors please add @LukasGardberg for code
@xuyxu
I've put up a pull request to add @LukasGardberg! :tada:
Okay, now hard majority has been moved to ops. I also added a simple test, and added soft voting to NeuralForestClassifier and SnapshotEnsembleClassifier, but they have not been tested.
Are there any other models I should add hard voting for? I'm assuming it's only applicable to classifiers right?
Also, I'm unsure if I solved the init inheritance problem properly, but the tests are at least passing now @xuyxu :)
Merged, thanks @LukasGardberg
Thanks a lot @xuyxu for all the help! Exciting making my first contribution :)
This is a first start to implementing hard majority voting, as mention in #118.
For a brief description of soft and hard voting, see sklearn's definition.
For hard voting, the vector
proba
returned by theforward
method of the VotingClassifier class now instead becomes a one-hot vector with a 1 on the class which had the majority of the base_estimators votes.Worth considering is that
torch.max
is called in theevaluate
method which essentially just gets the index of the 1 inproba
from theforward
method, which works, but maybe isn't a problem?Any suggestions for improvements are very welcome!