-
Notifications
You must be signed in to change notification settings - Fork 862
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
Move interpretable logic to InterpretableTabularPredictor #2981
Conversation
Job PR-2981-2955ec4 is done. |
2955ec4
to
e607559
Compare
Job PR-2981-e607559 is done. |
@@ -1 +1,2 @@ | |||
from .interpretable_predictor import InterpretableTabularPredictor | |||
from .predictor import TabularPredictor |
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.
nit: empty line at the EOF missing
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.
added
def fit(self, | ||
train_data, | ||
tuning_data=None, | ||
time_limit=None, | ||
*, | ||
presets='interpretable', | ||
fit_weighted_ensemble=False, | ||
calibrate=False, | ||
**kwargs): |
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.
nit: data types; add docstrings
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.
keeping to align with TabularPredictor, we can add type hints to TabularPredictor later on, but the type hints are already in TabularPredictor's docstring, which this method inherits.
for i in range(summaries.shape[0]): | ||
model_name = summaries.index.values[i] | ||
complexities.append(info['model_info'][model_name].get('complexity', np.nan)) | ||
summaries.insert(2, 'complexity', complexities) |
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.
why insert at specific position (2)? magic number needs an explanation
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.
updated
model_name = summaries_filtered.index.values[0] # best model is at top | ||
agmodel = self._trainer.load_model(model_name) | ||
imodel = agmodel.model | ||
print(imodel) |
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.
Use logging; print won't get into a file if filehandler is used
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.
keeping as print, we use print for leaderboard, and this is similar
52105ae
to
f4481c7
Compare
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.
LGTM
f4481c7
to
2ffdf82
Compare
Job PR-2981-2ffdf82 is done. |
Issue #, if available:
Description of changes:
Moved interpretable logic to its own predictor class
Removed non-interpretable models from the trained models in interpretable setting
Removed weighted ensemble in interpretable setting
Disabled calibration in interpretable setting
Added experimental warnings to interpretable setting and tutorial
DO NOT MERGE until new website stable is live, this code represents a breaking change in a tutorial
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.