Skip to content
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

Add XGBoost Regressor and Regression Pipeline #666

Merged
merged 11 commits into from Apr 16, 2020
Merged

Conversation

jeremyliweishih
Copy link
Contributor

@jeremyliweishih jeremyliweishih commented Apr 16, 2020

Fixes #528.

@codecov
Copy link

codecov bot commented Apr 16, 2020

Codecov Report

Merging #666 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #666      +/-   ##
==========================================
+ Coverage   99.02%   99.04%   +0.02%     
==========================================
  Files         135      139       +4     
  Lines        4709     4810     +101     
==========================================
+ Hits         4663     4764     +101     
  Misses         46       46              
Impacted Files Coverage Δ
evalml/pipelines/__init__.py 100.00% <ø> (ø)
evalml/pipelines/classification/catboost_binary.py 100.00% <ø> (ø)
...ml/pipelines/classification/catboost_multiclass.py 100.00% <ø> (ø)
evalml/pipelines/classification/xgboost_binary.py 100.00% <ø> (ø)
...lml/pipelines/classification/xgboost_multiclass.py 100.00% <ø> (ø)
evalml/pipelines/components/__init__.py 100.00% <ø> (ø)
evalml/pipelines/components/estimators/__init__.py 100.00% <ø> (ø)
evalml/pipelines/regression/catboost.py 100.00% <ø> (ø)
...tion_pipeline_tests/test_xgboost_classification.py 100.00% <ø> (ø)
...lines/components/estimators/regressors/__init__.py 100.00% <100.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d154ade...4f6e91d. Read the comment docs.

@jeremyliweishih jeremyliweishih self-assigned this Apr 16, 2020
@jeremyliweishih jeremyliweishih marked this pull request as ready for review Apr 16, 2020
@jeremyliweishih jeremyliweishih requested a review from dsherry Apr 16, 2020
SEED_MIN = -2**31
SEED_MAX = 2**31 - 1

def __init__(self, eta=0.1, max_depth=3, min_child_weight=1, n_estimators=100, random_state=0):
Copy link
Collaborator

@dsherry dsherry Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Are there any regression-specific params we should add now? I'm excited to do some tuning of this later lol

Copy link
Contributor Author

@jeremyliweishih jeremyliweishih Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see and understand the parameters between classification and regression should be the same! It has been a while since I've looked at boosted trees though 😄

Copy link
Collaborator

@dsherry dsherry Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's what I think too. This is great for now. Thanks!

@@ -59,6 +60,7 @@ def all_components():
import_or_raise("xgboost", error_msg="XGBoost not installed.")
except ImportError:
components.pop(XGBoostClassifier.name)
components.pop(XGBoostRegressor.name)
Copy link
Collaborator

@dsherry dsherry Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!! I can't wait to delete all this code... :)


def __init__(self, parameters, random_state=0):
super().__init__(parameters=parameters,
random_state=random_state)
Copy link
Collaborator

@dsherry dsherry Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this __init__ necessary?

Copy link
Contributor Author

@jeremyliweishih jeremyliweishih Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope! Will remove.

@dsherry
Copy link
Collaborator

dsherry commented Apr 16, 2020

I haven't read up on this but I think this is what Steve was talking about in the github workshop last week:

Screen Shot 2020-04-16 at 5 16 27 PM

@jeremyliweishih you did a git mv right?

@jeremyliweishih
Copy link
Contributor Author

jeremyliweishih commented Apr 16, 2020

@dsherry Not sure what you’re referring to but this was just renaming a file

@dsherry
Copy link
Collaborator

dsherry commented Apr 16, 2020

@jeremyliweishih lol yeah that's what I was asking. Question was totally not related to this PR 😂

Copy link
Collaborator

@dsherry dsherry left a comment

Good stuff!! ⛴️

@dsherry
Copy link
Collaborator

dsherry commented Apr 16, 2020

@jeremyliweishih attach this PR to #528 ?

@jeremyliweishih jeremyliweishih merged commit 9848eff into master Apr 16, 2020
2 checks passed
@dsherry dsherry deleted the 528_xgboost_reg branch Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add xgboost regression pipeline
3 participants