Add Vowpal Wabbit estimators #2846
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2846 +/- ##
=======================================
+ Coverage 99.7% 99.7% +0.1%
=======================================
Files 302 307 +5
Lines 28872 29049 +177
=======================================
+ Hits 28781 28958 +177
Misses 91 91
Continue to review full report at Codecov.
|
…abbit_integration
angela97lin
requested review from
christopherbunn,
freddyaboulton,
ParthivNaresh,
bchen1116,
chukarsten,
dsherry,
eccabay and
jeremyliweishih
There was a problem hiding this comment.
This looks all well and good to me! I have to say, I'm a little out of the loop on this classifier/regressor and mostly just thought "VowpalWabbit" was weird seeing it living on the PR list. But this looks like a textbook new component addition. I think it would be very interesting using this one as a guinea pig for @christopherbunn 's modifications to the perf test report.
| raise NotImplementedError( | ||
| "Feature importance is not implemented for the Vowpal Wabbit classifiers." | ||
| ) |
There was a problem hiding this comment.
I think what we normally do is return an array of 0's. I think this is fine for now, and ultimately the right thing, but we might have to revisit this again come AutoML integration.
|
Update: looks like the vowpal wabbit 8.8.1 version (aka the latest conda version) has a very different API from the latest (8.11.0) version which I based my code on. Filed VowpalWabbit/vowpal_wabbit#3406, but otherwise may consider looking into using the older API or holding off entirely. |
|
Since there's the discrepancy above, we could either:
Prophet is already a pip only package so it's not an entirely foreign concept, and I like this idea until we find it important to add to AutoML / better integrate it with EvalML. |
There was a problem hiding this comment.
@angela97lin Thank you for reviving this! I think it's ok to keep as pip-only. I'm curious what the perf tests will look like for this estimator. I just went down a documentation rabbit hole and there's a lot of options we can configure here.
| self, | ||
| loss_function="logistic", | ||
| learning_rate=0.5, | ||
| decay_learning_rate=1.0, |
There was a problem hiding this comment.
Great question. Hard to tell how this parameter is used from the docs but my guess is that if the decay learning rate is 1, the learning rate won't decay. That seems fine to me for a default, especially if we let automl tune it.
However, since the number of passes defaults to 1, I don't think the value of this parameter will take effect. @angela97lin Maybe we should also expose/tune the number of passes parameter?
…abbit_integration
…abbit_integration
|
@freddyaboulton I had done really preliminary perf testing a while back here: https://alteryx.atlassian.net/wiki/spaces/PS/pages/1059620252/Adding+Vowpal+Wabbit+to+AutoML I ran into a lot of weird behavior with the tuner and vowpal wabbit which will need to be resolved and will be pretty interesting to dig into once we want to add VW to AutoML, but you're totally right--there are so many knobs to turn with these estimators 😂 |
Reviving #770 as part of inno days experimentation.
(Closes #770)
Perf test results here on adding to AutoML: https://alteryx.atlassian.net/wiki/spaces/PS/pages/1059620252/Adding+Vowpal+Wabbit+to+AutoML
I don't think we should add this to AutoML quite yet, but it's still worth to have these estimators as components!
IIRC, we probably need to update the feedstock requirements to get the conda package tests to pass, will do, but wanted to put this up for review first 😁
Edit: looks like the vowpal wabbit 8.8.1 version (aka the latest conda version) has a very different API from the latest (8.11.0) version which I based my code on. Filed VowpalWabbit/vowpal_wabbit#3406, but otherwise may consider looking into using the older API or holding off entirely.