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] Fix R package cmake build command #5950

Closed
wants to merge 1 commit into from

Conversation

qinwf
Copy link

@qinwf qinwf commented Jun 27, 2023

No description provided.

@qinwf qinwf changed the title Fix R package cmake build command [R-package] Fix R package cmake build command Jun 27, 2023
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks for your interest in LightGBM.

Please update this with a description helping us understand how it improves the project. What problem does it fix? For example, can you share a specific command you can and an error you saw, which this fixes?

@qinwf
Copy link
Author

qinwf commented Jun 28, 2023

I got an error when building R packages with OpenCL on Linux.

 add in command-line arguments
# NOTE: build_r.R replaces the line below
command_line_args <- c("-DOpenCL_LIBRARY='/usr/local/cuda-10.2/targets/x86_64-linux/lib/libOpenCL.so.1.1'', '-DOpenCL_INCLUDE_DIR='/usr/local/cuda-10.2/targets/x86_64-linux/include'")

The generated install.libs.R is wrong.

@jameslamb
Copy link
Collaborator

Can you please share the exact command(s) you're using to run build the R package? So I can try to reproduce the error you saw, to be sure this is the right fix.

@qinwf
Copy link
Author

qinwf commented Jul 1, 2023

Rscript build_r.R --use-gpu --opencl-library=/usr/local/cuda/lib64/libOpenCL.so --opencl-include-dir=/usr/local/cuda/include

@jameslamb
Copy link
Collaborator

Thanks for that! When I run that command on master, like this:

Rscript build_r.R \
    --use-gpu \
    --opencl-library=/usr/local/cuda/lib64/libOpenCL.so \
    --opencl-include-dir=/usr/local/cuda/include

I see the error that I suspect you ran into

Error in source("install.libs.R", local = local.env) :
install.libs.R:143:77: unexpected string constant
142: # NOTE: build_r.R replaces the line below
143: command_line_args <- c('-DOpenCL_LIBRARY='/usr/local/cuda/lib64/libOpenCL.so''

And can confirm that this change fixes it on Linux.

Thanks for that! In the future, when you open pull requests here please provide enough information for us to be able to understand exactly how the change improves the project. And the text of error messages, like I've provided above, so others facing the same issue can find this PR from search.

However.... the proposed change exactly reverts #5607, which was introduced to fix similar quoting issues on Windows, reported in #5135. So if we merge this as-is, it'd again break support for building a GPU-enabled version of the R package on Windows.

Do you have access to a Windows environment to test running the following?

Rscript build_r.R \
    --use-gpu \
    --boost-root="C:\Users\James\repos\LightGBM\external_libs\compute" \
    --no-build-vignettes \
    --use-msys2

If not, then we'll have to close this PR and I will try to put up a fix soon that makes this work on both Unix-like operating system AND Windows.

Sorry it's such a manual process... we don't currently have CI jobs covering the GPU-enabled build of the R package (#3780).

@jameslamb
Copy link
Collaborator

Thanks for reporting this issue. I'll interpret your closing this without any comment as you saying "no, this is more complex than I expected and I don't want to help fix it".

I've opened #5960 to document the issue. You can subscribe there to be notified if someone picks it up and submits a fix.

Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants