Always shuffle data in default automl data split strategies #1265
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1265 +/- ##
=======================================
Coverage 99.93% 99.93%
=======================================
Files 207 208 +1
Lines 13157 13211 +54
=======================================
+ Hits 13149 13203 +54
Misses 8 8
Continue to review full report at Codecov.
|
dsherry
requested review from
angela97lin,
freddyaboulton,
bchen1116,
gsheni,
jeremyliweishih,
christopherbunn and
eccabay
| # if shuffle is disabled, the mean value learned on each CV fold's training data will be incredible inaccurate, | ||
| # thus yielding an R^2 well below 0. | ||
|
|
||
| n = 100000 |
There was a problem hiding this comment.
If this test doesn't take too long it might be worth running it with n=1000 so that the KFold CV is used?
There was a problem hiding this comment.
Ah shoot you're right. I'll track this down. I upped the number because I wasn't reproing it as reliably at lower values.
| @@ -645,6 +645,32 @@ def generate_fake_dataset(rows): | |||
| assert automl.data_split.test_size == (automl._LARGE_DATA_PERCENT_VALIDATION) | |||
|
|
|||
|
|
|||
| def test_data_split_shuffle(): | |||
| @@ -5,7 +5,7 @@ | |||
| class TrainingValidationSplit(BaseCrossValidator): | |||
| """Split the training data into training and validation sets""" | |||
|
|
|||
| def __init__(self, test_size=None, train_size=None, shuffle=True, stratify=None, random_state=0): | |||
| def __init__(self, test_size=None, train_size=None, shuffle=False, stratify=None, random_state=0): | |||
There was a problem hiding this comment.
Should this be shuffle=True?
There was a problem hiding this comment.
@gsheni I updated this to be the same as the other sklearn-defined CV methods, which have shuffle=False by default. Then, in the automl code which calls this, I set shuffle=True. I just did it to be consistent with sklearn.
dsherry
force-pushed
the
ds_1259_shuffle_data
branch
from
e3887a4
to
935bb1c
Compare
Fix #1259