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

[R-package] Support trees with linear models at leaves #3319

Closed
btrotta opened this issue Aug 19, 2020 · 3 comments · Fixed by #3699
Closed

[R-package] Support trees with linear models at leaves #3319

btrotta opened this issue Aug 19, 2020 · 3 comments · Fixed by #3699

Comments

@btrotta
Copy link
Collaborator

btrotta commented Aug 19, 2020

PR #3299 implements trees with linear models at the leaves. I have only tested this with the Python package, and it's possible that some additional changes are needed to get it working for R. To calculate the linear model, we need a Dataset object that contains the full feature data (for numerical features, not needed for categorical), rather than just the binned data. Therefore, when we create the dataset we need to know whether linear_tree is True. In the Python interface, we first create the dataset, then call lgb.train as follows:

train_data = lgb.Dataset(x)
params = {'linear_tree'=True}
est =  lgb.train(params, train_data)

So Python doesn't know the dataset parameters (e.g. linear_tree, or max_bin, etc), until we call lgb.train. This works because the dataset is initialised in the first line, but the data isn't actually loaded until lgb.train is called. The code that handles this seems quite complex and delicate, and I had to make a couple of small changes to get it to work properly (e.g. to use the refit functionality on an existing model). So I imagine some similar tweaks would be required for the R interface.

Also, the new code requires the Eigen library for linear algebra, which is causing some issues with the R code checks in CI. See this comment for details: #3299 (comment)

@jameslamb
Copy link
Collaborator

Thanks @btrotta ! If no one else picks this up before me, I'll come back and get it at some point.

For the issue with diagnostic-suppressing pragmas, we might have to do something like we do for the LightGBM ones:

echo "Removing unknown pragmas in headers"

@jameslamb
Copy link
Collaborator

I just added this to #2302 in the R package section, so based on our convention I'll close it.

Anyone reading this issue...please leave a comment if you'd like to create a pull request for it and I can re-open it!

@jameslamb
Copy link
Collaborator

When this is picked up, it should also get changes similar to #3685.

  1. build_r.R and build-cran-package.sh should be modified to only copy in the specific parts of Eigen that are needed (see MANIFEST.in changes in [python-package] remove unused Eigen files, compile with EIGEN_MPL2_ONLY (fixes #3684) #3685)
  2. -DEIGEN_MPL2_ONLY should be added to LGB_CPPFLAGS in R-package/configure.ac and R-package/configure.win

@jameslamb jameslamb reopened this Dec 31, 2020
StrikerRUS pushed a commit that referenced this issue Jan 18, 2021
…3319) (#3699)

* [R-package] enable use of trees with linear models at leaves (fixes #3319)

* remove problematic pragmas

* fix tests

* try to fix build scripts

* try fixing pragma check

* more pragma checks

* ok fix pragma stuff for real

* empty commit

* regenerate documentation

* try skipping test

* uncomment CI

* add note on missing value types for R

* add tests on saving and re-loading booster
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants