Implementation of unit testing

[yutayamazaki]

• I recommend you to implement unit testing to develop this package effectively and safely.
• I found RuntimeError in NGBRegresor and NGBClassifier noted in this Scikit-Learn source

[avati]

@yutayamazaki could you describe what error you got with the two classes?

[tonyduan]

I'm able to reproduce this issue by attempting to run the file [1]. The issue seems to arise due to the way Scikit-Learn is using GridSearchCV; any BaseEstimator sub-class must be explicit about its parameters and specifically should not take **kwargs or *args.

My two cents on the Scikit-Learn API integration:

• Seems like the main advantage of this integration is easier tuning with GridSearchCV. To keep this possible we'll have to copy/paste NGBoost's __init__ parameters (Dist, Score, etc) into each of NGBRegressor and NGBClassifier. This copy/pasting isn't a huge deal in my opinion.

• However, the sub-classing of ClassifierMixin and RegressorMixin (or sksurv's SurvivalAnalysisMixin) yields objectives for cross-validation that feel a bit unnatural (specifically, accuracy, R^2 score, and concordance) in our distributional regression context. I'm thinking of implementing a NLLMixin that all can be used for all cases.

Edit: this has been implemented and should be working now, see examples/sklearn_cv.py.

[1] https://github.com/stanfordmlgroup/ngboost/blob/master/examples/empirical/clf_sklearn.py

[wakame1367]

#35

[alejandroschuler]

note to get CI up and running when we have some better tests: https://github.com/stanfordmlgroup/ngboost/issues/36

[alejandroschuler]

I also want to try out pytest instead of unittest- seems cleaner.

[alejandroschuler]

note to add tests for every combination of distribution and scoring rule

[alejandroschuler]

note to add tests for sample weighting

[wakame1367]

I also want to try out pytest instead of unittest- seems cleaner.

Should I rewrite tests from unittest to tests using pytest?

[alejandroschuler]

@wakamezake sure, it would be welcome! I don't have any experience with either but since we're just getting started it's a good time to pick a framework and pytest seems a little easier.

[alejandroschuler]

hey @wakamezake I've started to expand the test suite a bit and I'm wondering if you're interested in lending a hand. Ideally I'd like to test all the combinations of all of the features that we've laid out in the vignette in a way that's similar to how you wrote the tests in test_basic.py and what I have now in test_distns.py, although in a way that's modular and doesn't require a lot of code copy-pasting. Ideally we would also unit test some of the low-level features like what's described in the developer guide and even the low-level helper methods in ngboost.py. At a first pass, though, ensuring that all of the high-level features work with each other without throwing errors would be good enough.

Is that something you (or anyone else here) would be interested in working on?

Thanks!

[alejandroschuler]

https://hypothesis.readthedocs.io/en/latest/quickstart.html

[ryan-wolbeck]

hey @wakamezake I've started to expand the test suite a bit and I'm wondering if you're interested in lending a hand. Ideally I'd like to test all the combinations of all of the features that we've laid out in the vignette in a way that's similar to how you wrote the tests in test_basic.py and what I have now in test_distns.py, although in a way that's modular and doesn't require a lot of code copy-pasting. Ideally we would also unit test some of the low-level features like what's described in the developer guide and even the low-level helper methods in ngboost.py. At a first pass, though, ensuring that all of the high-level features work with each other without throwing errors would be good enough.

Is that something you (or anyone else here) would be interested in working on?

Thanks!

Hey @alejandroschuler did these links move somewhere else?

[alejandroschuler]

@ryan-wolbeck good question- both the vignette and developer guide were rolled into the one-stop-shop user guide, so neither exist anymore.