-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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] 32-bit library support #3187
Comments
I've updated #2302 , but I'd like to leave this open for a bit to discuss. I believe it is worth calling this issue |
I remember we enabled optional building for 32bit in Python-package some time ago. The only change that was needed |
oh cool! Hey that's at least something simple I can try, thank you! |
It was kind of hard to figure out how to replicate the 32-bit R builds. The Just noting for myself here that this is the command to use to test if the package works on 32-bit Windows: sh build-cran-package.sh
"C:\Program Files\R\R-4.0.2\bin\i386\R.exe" CMD check lightgbm_3.0.0-1.tar.gz --as-cran You'll know this works if you see a message in the logs saying the library is being built on a 32-bit platform, like this
|
I can confirm that the package is installable with 32-bit R on Windows
However, the tests are failing |
here are the full logs from the tests. All failed with this error log
|
Hmmm, how is it possible that according to the R Hub we get only one NOTE for 32 bit Windows?
|
Unfortunately the logs link from that R Hub build are no longer available, but if they were you'd see a line like this:
It's a frustrating part of using R Hub, it doesn't exactly replicate CRAN as far as I can tell, even when you pass Honestly, maybe at this point it makes sense to just try submitting to CRAN and see what they say? I think we are close enough that it's worth a first attempt. Then we'll know for sure what needs to be done. @guolinke since you are the maintainer in You just have to upload the package here (https://cran.r-project.org/submit.html), and then they'll send you an email (usually within 48 hours), with some description of issues and links to logs from builds. You can use this source distribution of the package that I just created from |
in the meantime I'll submit to win-builder and share the full logs here once they're done, so we can see where we stand with that service. sh build-cran-package.sh devtools::check_win_release(
pkg = "lightgbm_r/",
args = NULL,
manual = TRUE,
email = "[email protected]",
quiet = FALSE,
) |
here is the log from win-build. It shows the error I was mentioning. 00check.log
00install.out
I still think it is a good idea to try submitting to CRAN and see what happens. |
ooooooo this discussion looks promising! https://community.rstudio.com/t/configure-win-and-cran-submission/24684
I think I misunderstood the "Biarch" field. Let me try re-submitting with |
here are the full results, by the way. This link will work for 72 hours https://win-builder.r-project.org/oUfPEaiEOrWF/ |
ok new logs after updating to 00check.log
00install.out
Full record is here: https://win-builder.r-project.org/g37i74xkiEnW/ This is encouraging because it means:
|
I did not intend to close this in #3285 |
The issues I was seeing on CRAN logs00check.log
00install.out
|
@jameslamb |
I'm really not sure how strongly CRAN feels about 32-bit. So here's my proposal.
|
@jameslamb BTW, it seems the 32-bit Linux build can pass? |
BTW, it seems the Windows version is built by MinGW? I remember @Laurae2 benchmarked that MinGW is much slower than vc++ in windows. And 32-bit version should work by vc++ build. |
Oh sure, I understand that we would not advise anyone to use 32-bit and would tell them they'll get better performance on 64-bit. I haven't tried 32-bit with MSVC yet (like #3187 (comment)), just because you have to use MINGW on CRAN. I will try it tonight, based on #3187 (comment). In my opinion, the value of this issue isn't that any of our users would ever actually use the 32-bit version, but just that it's an obstacle we have to get past to be on CRAN. A lot of R users will not be comfortable enough with git or CMake to build the package from source, and other packages in the R ecosystem like If we follow my proposal in #3187 (comment), I think it could satisfy CRAN to say that that 32-bit package is much more limited. Like maybe we can limit it to only training on dense matrices, no distributed training, etc. Maybe that would cut down on the maintenance burden at least? |
I don't think so. I think the only two R Hub 32-bit Linux builds were on Solaris and those both failed (#629 (comment)) |
@jameslamb |
yep I agree! I just think that having a not-that-great 32-bit build is worth it if it gets the R package onto CRAN. I'll report the results from building a 32-bit version with MSVC here shortly, let's see how it goes. |
I saw the errors in 32-bit version are caused by |
Ah yes, this is very confusing. See this explanation when @StrikerRUS asked me the same question 😬 (#3187 (comment)). Those Windows jobs have names like |
A quick summary:
|
thank you, yes I agree with that! With the one condition CRAN might reject our submission with the skipped tests. But we'll just have to see. I'll open a PR with the skipping once I get working results from win-builder. |
Ok I just tried with MSVC. You can see the changes I had to make here: https://github.com/microsoft/LightGBM/compare/master...jameslamb:r/try-32bit-msvc?expand=1. I ran this "C:\Program Files\R\R-4.0.2\bin\i386\Rscript.exe" build_r.R Here's as much log as I was able to copy: build_r.R logs
There were a lot of warnings and linker errors. here's a small sample
|
I might be able to fix that, though. I'm experimenting with some things. |
Happy to report that skipping the tests does allow us to pass checks on 32-bit Windows! I've opened #3302 with those changes. |
What do you think about the possible solution I proposed with borrowing |
Coming from #3302 (comment), @guolinke and @shiyu1994 here is how to develop on this. These steps assume you are on a 64-bit Windows system (but will use the 32-bit version of R). Set up R for Windows
Build the CRAN package
Try installing and building with 64-bit R
R CMD check --as-cran --no-multiarch lightgbm_3.0.0-1.tar.gz Try with 32-bit R
Some Notes
Let me know if you have any questions, and thank you so much for your efforts! |
I first build the 32-bit cli/python versions, both of them can work. |
ooooo that is really encouraging! That might mean there is just something we need to change in the R-specific files in Can you share the command(s) for how you built the python package with 32 bit? |
@jameslamb it is quite simple.
|
Summary
As I've investigated #2960 as part of #629 , I've learned that packages on CRAN have to be able to build on 32-bit Windows.
I believe that we need to add support for a 32-bit build LightGBM to get the R package to CRAN.
Evidence that this is required
I have not yet found any official statement that says "all R packages have to build on 32-bit Windows", but I see strong evidence that that is true.
You can find the current list of check environments at https://cran.r-project.org/web/checks/check_flavors.html. None of those seem to be 32-bit
but in "Writing R Extensions" I see the following:
And in the "R for Windows FAQs":
This is why you see pull requests this in other projects: dmlc/xgboost#5777
I've tried looking in the source for R CMD check but it is very difficult to follow.
Motivation
I believe that we need to add support for a 32-bit build LightGBM to get the R package to CRAN.
Description
To check if the R package works with 32-bit R, try the following:
sh build-cran-package.sh "C:\Program Files\R\R-4.0.2\bin\i386\R.exe" CMD check lightgbm_3.0.0-1.tar.gz --as-cran
References
There was some discussion about this in #1129 , where it was decided that supporting a 32-bit is possible but that the maintenance burden would be too much. Given that it is needed to get LightGBM to CRAN, I think it should be reconsidered.
The text was updated successfully, but these errors were encountered: