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

xgboost: expose watchlist and callbacks #1859

Merged
merged 2 commits into from
Jun 22, 2017

Conversation

khotilov
Copy link
Contributor

  • expose watchlist and callbacks through parameters classif.xgboost - error if early.stop.round is set? #1264
  • remove silent from the parameters - it's overridden by verbose in xgboost R interface and has no effect
  • set default lambda=1 as it is such for tree boosters
    • Question: is there any way to conditionally set default lambda=0 when booster=='gblinear'?
  • add tweedie_variance_power parameter
  • placeholders for some recently added parameters (commented out until the next CRAN release)

…et default lambda=1; add tweedie_variance_power param
@lintr-bot
Copy link

R/RLearner_classif_xgboost.R:43:7: style: TODO comments should be removed.

# TODO: uncomment the following after the next CRAN update, and set max_depth's lower = 0L
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

R/RLearner_regr_xgboost.R:43:7: style: TODO comments should be removed.

# TODO: uncomment the following after the next CRAN update, and set max_depth's lower = 0L
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@larskotthoff
Copy link
Member

@mb706 Can we have the lintr allow TODO comments please?

@mb706
Copy link
Contributor

mb706 commented Jun 20, 2017 via email

@larskotthoff
Copy link
Member

That style point doesn't apply here -- it's a todo for when a new package version is released. The style guide doesn't say anything about not allowing TODOs in general.

@khotilov
Copy link
Contributor Author

TODO codetags have fairly different semantics (informal tasks pending completion) to FIXME's (something bad or suspicious that needs cleanup, usually before a release). To me, the style guide's p17 simply emphasizes this distinction, even if in a potentially confusing way.

@larskotthoff
Copy link
Member

@khotilov Could you please disable the TODO lintr as suggested so we can see if the build succeeds then?

@khotilov
Copy link
Contributor Author

@larskotthoff The build has passed.

@larskotthoff
Copy link
Member

Thanks! Merging.

@larskotthoff larskotthoff merged commit f7786ef into mlr-org:master Jun 22, 2017
mllg pushed a commit that referenced this pull request Jul 11, 2017
* xgboost: expose watchlist and callbacks; remove silent from params; set default lambda=1; add tweedie_variance_power param

* disable TODO linter
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.

4 participants