-
Notifications
You must be signed in to change notification settings - Fork 729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix automl handling of positional initializer args #373
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,10 +45,10 @@ | |
AutomatedKernelDML = addAutomatedML(KernelDML) | ||
AutomatedNonParamDML = \ | ||
addAutomatedML(NonParamDML) | ||
AutomatedForestDML = addAutomatedML(ForestDML) | ||
AutomatedCausalForestDML = addAutomatedML(CausalForestDML) | ||
|
||
AUTOML_SETTINGS_REG = { | ||
'experiment_timeout_minutes': 1, | ||
'experiment_timeout_minutes': 15, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 15 minutes is the new lowest allowed value for the experiment timeout. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, that’s a lot |
||
'enable_early_stopping': True, | ||
'iteration_timeout_minutes': 1, | ||
'max_cores_per_iteration': 1, | ||
|
@@ -61,7 +61,7 @@ | |
} | ||
|
||
AUTOML_SETTINGS_CLF = { | ||
'experiment_timeout_minutes': 1, | ||
'experiment_timeout_minutes': 15, | ||
'enable_early_stopping': True, | ||
'iteration_timeout_minutes': 1, | ||
'max_cores_per_iteration': 1, | ||
|
@@ -118,7 +118,7 @@ def automl_model_sample_weight_reg(): | |
|
||
|
||
@pytest.mark.automl | ||
class TestAutomatedDML(unittest.TestCase): | ||
class TestAutomatedML(unittest.TestCase): | ||
|
||
@classmethod | ||
def setUpClass(cls): | ||
|
@@ -134,7 +134,6 @@ def setUpClass(cls): | |
|
||
def test_nonparam(self): | ||
"""Testing the completion of the fit and effect estimation of an automated Nonparametic DML""" | ||
Y, T, X, _ = ihdp_surface_B() | ||
est = AutomatedNonParamDML(model_y=automl_model_reg(), | ||
model_t=automl_model_clf(), | ||
model_final=automl_model_sample_weight_reg(), featurizer=None, | ||
|
@@ -144,7 +143,6 @@ def test_nonparam(self): | |
|
||
def test_param(self): | ||
"""Testing the completion of the fit and effect estimation of an automated Parametric DML""" | ||
Y, T, X, _ = ihdp_surface_B() | ||
est = AutomatedLinearDML(model_y=automl_model_reg(), | ||
model_t=GradientBoostingClassifier(), | ||
featurizer=None, | ||
|
@@ -154,28 +152,21 @@ def test_param(self): | |
|
||
def test_forest_dml(self): | ||
"""Testing the completion of the fit and effect estimation of an AutomatedForestDML""" | ||
|
||
Y, T, X, _ = ihdp_surface_B() | ||
est = AutomatedForestDML(model_y=automl_model_reg(), | ||
model_t=GradientBoostingClassifier(), | ||
discrete_treatment=True, | ||
n_estimators=1000, | ||
subsample_fr=.8, | ||
min_samples_leaf=10, | ||
min_impurity_decrease=0.001, | ||
verbose=0, min_weight_fraction_leaf=.01) | ||
est = AutomatedCausalForestDML(model_y=automl_model_reg(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where did the data generation go? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We were already performing the data generation in the big |
||
model_t=GradientBoostingClassifier(), | ||
discrete_treatment=True, | ||
n_estimators=1000, | ||
max_samples=.4, | ||
min_samples_leaf=10, | ||
min_impurity_decrease=0.001, | ||
verbose=0, min_weight_fraction_leaf=.01) | ||
est.fit(Y, T, X=X) | ||
_ = est.effect(X) | ||
|
||
|
||
@pytest.mark.automl | ||
class TestAutomatedMetalearners(unittest.TestCase): | ||
|
||
def test_TLearner(self): | ||
"""Testing the completion of the fit and effect estimation of an AutomatedTLearner""" | ||
# TLearner test | ||
# Instantiate TLearner | ||
Y, T, X, _ = ihdp_surface_B() | ||
est = AutomatedTLearner(models=automl_model_reg()) | ||
|
||
# Test constant and heterogeneous treatment effect, single and multi output y | ||
|
@@ -188,7 +179,6 @@ def test_SLearner(self): | |
# Test constant treatment effect with multi output Y | ||
# Test heterogeneous treatment effect | ||
# Need interactions between T and features | ||
Y, T, X, _ = ihdp_surface_B() | ||
est = AutomatedSLearner(overall_model=automl_model_reg()) | ||
|
||
est.fit(Y, T, X=X) | ||
|
@@ -206,3 +196,20 @@ def test_DALearner(self): | |
|
||
est.fit(Y, T, X=X) | ||
_ = est.effect(X) | ||
|
||
def test_positional(self): | ||
"""Test that positional arguments can be used with AutoML wrappers""" | ||
|
||
class TestEstimator: | ||
def __init__(self, model_x): | ||
self.model_x = model_x | ||
|
||
def fit(self, X, Y): | ||
self.model_x.fit(X, Y) | ||
return self | ||
|
||
def predict(self, X): | ||
return self.model_x.predict(X) | ||
|
||
AutoMLTestEstimator = addAutomatedML(TestEstimator) | ||
AutoMLTestEstimator(automl_model_reg()).fit(X, Y).predict(X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the prefix actually used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and what if "arg0" is also a keyword argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it is just used to make it easier for a person to identify the experiment when looking through experiments in AzureML studio, and that the service itself adds a unique suffix so that the whole thing will be unambiguous in any case.