merged 'master' into r/msvc-support

.appveyor.yml

@@ -8,7 +8,9 @@ configuration:  # a trick to construct a build matrix with multiple Python versi
 environment:
   matrix:
     - COMPILER: MSVC
+      TASK: python
     - COMPILER: MINGW
+      TASK: python
 
 clone_depth: 5
 
@@ -17,41 +19,20 @@ install:
   - set PATH=%PATH:C:\Program Files\Git\usr\bin;=%  # delete sh.exe from PATH (mingw32-make fix)
   - set PATH=C:\mingw-w64\x86_64-8.1.0-posix-seh-rt_v6-rev0\mingw64\bin;%PATH%
   - set PYTHON_VERSION=%CONFIGURATION%
+  - set CONDA_ENV="test-env"
   - ps: >-
       switch ($env:PYTHON_VERSION) {
-          "2.7" {$env:MINICONDA = """C:\Miniconda-x64"""}
-          "3.5" {$env:MINICONDA = """C:\Miniconda35-x64"""}
-          "3.6" {$env:MINICONDA = """C:\Miniconda36-x64"""}
-          "3.7" {$env:MINICONDA = """C:\Miniconda37-x64"""}
-          default {$env:MINICONDA = """C:\Miniconda37-x64"""}
+          "2.7" {$env:MINICONDA = "C:\Miniconda-x64"}
+          "3.5" {$env:MINICONDA = "C:\Miniconda35-x64"}
+          "3.6" {$env:MINICONDA = "C:\Miniconda36-x64"}
+          "3.7" {$env:MINICONDA = "C:\Miniconda37-x64"}
+          default {$env:MINICONDA = "C:\Miniconda37-x64"}
       }
-  - set PATH=%MINICONDA%;%MINICONDA%\Scripts;%PATH%
-  - ps: $env:LGB_VER = (Get-Content VERSION.txt).trim()
-  - activate
-  - conda config --set always_yes yes --set changeps1 no
-  - conda update -q -y conda
-  - conda create -q -y -n test-env python=%PYTHON_VERSION% joblib matplotlib numpy pandas psutil pytest python-graphviz "scikit-learn<=0.21.3" scipy
-  - activate test-env
+      $env:PATH="$env:MINICONDA;$env:MINICONDA\Scripts;$env:PATH"
+  - ps: $env:LGB_VER = (Get-Content $env:APPVEYOR_BUILD_FOLDER\VERSION.txt).trim()
 
-build_script:
-  - cd %APPVEYOR_BUILD_FOLDER%\python-package
-  - IF "%COMPILER%"=="MINGW" (
-    python setup.py install --mingw)
-    ELSE (
-    python setup.py install)
+build: false
 
 test_script:
-  - pytest %APPVEYOR_BUILD_FOLDER%\tests\python_package_test
-  - cd %APPVEYOR_BUILD_FOLDER%\examples\python-guide
-  - ps: >-
-      @("import matplotlib", "matplotlib.use('Agg')") + (Get-Content "plot_example.py") | Set-Content "plot_example.py"  # prevent interactive window mode
-      (Get-Content "plot_example.py").replace('graph.render(view=True)', 'graph.render(view=False)') | Set-Content "plot_example.py"
-  - ps: >-
-      foreach ($file in @(Get-ChildItem *.py)) {
-        @("import sys, warnings", "warnings.showwarning = lambda message, category, filename, lineno, file=None, line=None: sys.stdout.write(warnings.formatwarning(message, category, filename, lineno, line))") + (Get-Content $file) | Set-Content $file
-        python $file
-        if (!$?) { $host.SetShouldExit(-1) }
-      }  # run all examples
-  - cd %APPVEYOR_BUILD_FOLDER%\examples\python-guide\notebooks
-  - conda install -q -y -n test-env ipywidgets notebook
-  - jupyter nbconvert --ExecutePreprocessor.timeout=180 --to notebook --execute --inplace *.ipynb  # run all notebooks
+  - conda init powershell
+  - powershell.exe -ExecutionPolicy Bypass -File %APPVEYOR_BUILD_FOLDER%\.ci\test_windows.ps1

.ci/test_r_package.sh

@@ -91,7 +91,7 @@ if grep -q -R "WARNING" "$LOG_FILE_NAME"; then
     exit -1
 fi
 
-ALLOWED_CHECK_NOTES=3
+ALLOWED_CHECK_NOTES=2
 NUM_CHECK_NOTES=$(
     cat ${LOG_FILE_NAME} \
         | grep -e '^Status: .* NOTE.*' \

.ci/test_windows.ps1

@@ -6,6 +6,20 @@ function Check-Output {
   }
 }
 
+# unify environment variables for Azure devops and AppVeyor
+if (Test-Path env:APPVEYOR) {
+  $env:APPVEYOR = "true"
+  $env:BUILD_SOURCESDIRECTORY = $env:APPVEYOR_BUILD_FOLDER
+}
+
+# setup for Python
+conda init powershell
+conda activate
+conda config --set always_yes yes --set changeps1 no
+conda update -q -y conda
+conda create -q -y -n $env:CONDA_ENV python=$env:PYTHON_VERSION joblib matplotlib numpy pandas psutil pytest python-graphviz "scikit-learn<=0.21.3" scipy ; Check-Output $?
+conda activate $env:CONDA_ENV
+
 if ($env:TASK -eq "regular") {
   mkdir $env:BUILD_SOURCESDIRECTORY/build; cd $env:BUILD_SOURCESDIRECTORY/build
   cmake -A x64 .. ; cmake --build . --target ALL_BUILD --config Release ; Check-Output $?
@@ -33,15 +47,27 @@ elseif ($env:TASK -eq "bdist") {
   python setup.py bdist_wheel --plat-name=win-amd64 --universal ; Check-Output $?
   cd dist; pip install @(Get-ChildItem *.whl) ; Check-Output $?
   cp @(Get-ChildItem *.whl) $env:BUILD_ARTIFACTSTAGINGDIRECTORY
+} elseif (($env:APPVEYOR -eq "true") -and ($env:TASK -eq "python")) {
+  cd $env:BUILD_SOURCESDIRECTORY\python-package
+  if ($env:COMPILER -eq "MINGW") {
+    python setup.py install --mingw ; Check-Output $?
+  } else {
+    python setup.py install ; Check-Output $?
+  }
 }
 
-$tests = $env:BUILD_SOURCESDIRECTORY + $(If ($env:TASK -eq "sdist") {"/tests/python_package_test"} Else {"/tests"})  # cannot test C API with "sdist" task
+if (($env:TASK -eq "sdist") -or (($env:APPVEYOR -eq "true") -and ($env:TASK -eq "python"))) {
+  $tests = $env:BUILD_SOURCESDIRECTORY + "/tests/python_package_test"
+} else {
+  # cannot test C API with "sdist" task
+  $tests = $env:BUILD_SOURCESDIRECTORY + "/tests"
+}
 pytest $tests ; Check-Output $?
 
-if ($env:TASK -eq "regular") {
+if (($env:TASK -eq "regular") -or (($env:APPVEYOR -eq "true") -and ($env:TASK -eq "python"))) {
   cd $env:BUILD_SOURCESDIRECTORY/examples/python-guide
   @("import matplotlib", "matplotlib.use('Agg')") + (Get-Content "plot_example.py") | Set-Content "plot_example.py"
-  (Get-Content "plot_example.py").replace('graph.render(view=True)', 'graph.render(view=False)') | Set-Content "plot_example.py"
+  (Get-Content "plot_example.py").replace('graph.render(view=True)', 'graph.render(view=False)') | Set-Content "plot_example.py"  # prevent interactive window mode
   foreach ($file in @(Get-ChildItem *.py)) {
     @("import sys, warnings", "warnings.showwarning = lambda message, category, filename, lineno, file=None, line=None: sys.stdout.write(warnings.formatwarning(message, category, filename, lineno, line))") + (Get-Content $file) | Set-Content $file
     python $file ; Check-Output $?

.travis.yml

@@ -45,11 +45,11 @@ before_install:
   - if [[ $TRAVIS_OS_NAME == "osx" ]]; then
         export OS_NAME="macos";
         export COMPILER="gcc";
-        export R_MAC_VERSION=3.6.1;
+        export R_MAC_VERSION=3.6.3;
     else
         export OS_NAME="linux";
         export COMPILER="clang";
-        export R_TRAVIS_LINUX_VERSION=3.6.1-3bionic;
+        export R_TRAVIS_LINUX_VERSION=3.6.3-1bionic;
     fi
   - export CONDA="$HOME/miniconda"
   - export PATH="$CONDA/bin:$PATH"

.vsts-ci.yml

@@ -99,7 +99,7 @@ jobs:
       echo "##vso[task.setvariable variable=CONDA]$CONDA"
       echo "##vso[task.prependpath]$CONDA/bin"
       echo "##vso[task.setvariable variable=JAVA_HOME]$JAVA_HOME_8_X64"
-      echo "##vso[task.setvariable variable=R_MAC_VERSION]3.6.1"
+      echo "##vso[task.setvariable variable=R_MAC_VERSION]3.6.3"
     displayName: 'Set variables'
   - bash: $(Build.SourcesDirectory)/.ci/setup.sh
     displayName: Setup
@@ -108,7 +108,7 @@ jobs:
   - task: PublishBuildArtifacts@1
     condition: and(succeeded(), in(variables['TASK'], 'regular', 'sdist', 'bdist'), not(startsWith(variables['Build.SourceBranch'], 'refs/pull/')))
     inputs:
-      pathtoPublish: '$(Build.ArtifactStagingDirectory)' 
+      pathtoPublish: '$(Build.ArtifactStagingDirectory)'
       artifactName: PackageAssets
       artifactType: container
 ###########################################
@@ -129,11 +129,13 @@ jobs:
         TASK: bdist
         PYTHON_VERSION: 3.5
   steps:
-  - powershell: Write-Host "##vso[task.prependpath]$env:CONDA\Scripts"
-    displayName: Enable conda
+  - powershell: |
+      Write-Host "##vso[task.prependpath]$env:CONDA\Scripts"
+      Write-Host "##vso[task.setvariable variable=AZURE]true"
+    displayName: 'Set Variables'
   - script: |
-      cmd /c 'activate & conda config --set always_yes yes --set changeps1 no & conda update -q -y conda & conda create -q -y -n %CONDA_ENV% python=%PYTHON_VERSION% joblib matplotlib numpy pandas psutil pytest python-graphviz "scikit-learn<=0.21.3" scipy'
-      cmd /c "activate %CONDA_ENV% & powershell -ExecutionPolicy Bypass -File %BUILD_SOURCESDIRECTORY%/.ci/test_windows.ps1"
+      cmd /c "conda init powershell"
+      cmd /c "powershell -ExecutionPolicy Bypass -File %BUILD_SOURCESDIRECTORY%/.ci/test_windows.ps1"
     displayName: Test
   - task: PublishBuildArtifacts@1
     condition: and(succeeded(), in(variables['TASK'], 'regular', 'sdist', 'bdist'), not(startsWith(variables['Build.SourceBranch'], 'refs/pull/')))

CMakeLists.txt

@@ -235,7 +235,15 @@ file(GLOB SOURCES
 )
 
 add_executable(lightgbm src/main.cpp ${SOURCES})
-add_library(_lightgbm SHARED src/c_api.cpp src/lightgbm_R.cpp ${SOURCES})
+list(APPEND SOURCES "src/c_api.cpp")
+
+# Only build the R part of the library if building for
+# use with the R package
+if(BUILD_FOR_R)
+  list(APPEND SOURCES "src/lightgbm_R.cpp")
+endif(BUILD_FOR_R)
+
+add_library(_lightgbm SHARED ${SOURCES})
 
 if(MSVC)
     set_target_properties(_lightgbm PROPERTIES OUTPUT_NAME "lib_lightgbm")

R-package/.Rbuildignore

@@ -4,20 +4,22 @@
 ^pkgdown$
 
 # Objects created by compilation
-\.o$
-\.so$
-\.dll$
-\.out$
-\.bin$
+^.*\.o
+^.*\.so
+^.*\.dll
+^.*\.out
+^.*\.bin
 
 # Code copied in at build time
 ^src/CMakeLists.txt$
+^Makefile$
+^src/build/.*$
 
 # unnecessary files from submodules
 ^src/compute/.appveyor.yml$
 ^src/compute/.coveralls.yml$
 ^src/compute/.travis.yml$
-^src/compute/test/$
+^src/compute/test/.*$
 ^src/compute/index.html$
 ^src/compute/.git$
 ^src/compute/.gitignore$

R-package/src/install.libs.R

@@ -43,7 +43,7 @@ if (!use_precompile) {
   # Prepare installation steps
   cmake_cmd <- "cmake "
   build_cmd <- "make _lightgbm"
-  lib_folder <- file.path(R_PACKAGE_SOURCE, "src", fsep = "/")
+  lib_folder <- file.path(source_dir, fsep = "/")
 
   if (use_gpu) {
     cmake_cmd <- paste0(cmake_cmd, " -DUSE_GPU=ON ")
@@ -98,7 +98,7 @@ if (!use_precompile) {
       } else {
         cmake_cmd <- paste0(cmake_cmd, local_vs_def)
         build_cmd <- "cmake --build . --target _lightgbm --config Release"
-        lib_folder <- file.path(R_PACKAGE_SOURCE, "src/Release", fsep = "/")
+        lib_folder <- file.path(source_dir, "Release", fsep = "/")
       }
     }
   }
@@ -110,9 +110,7 @@ if (!use_precompile) {
   # Makefile. We don't need it here anyway since targets are built serially, so trying
   # to remove it with this hack
   generated_makefile <- file.path(
-    R_PACKAGE_SOURCE
-    , "src"
-    , "build"
+    build_dir
     , "Makefile"
   )
   if (file.exists(generated_makefile)) {
@@ -163,12 +161,21 @@ if (!use_precompile) {
   }
 }
 
-# Check installation correctness
+# Packages with install.libs.R need to copy some artifacts into the
+# expected places in the package structure.
+# see https://cran.r-project.org/doc/manuals/r-devel/R-exts.html#Package-subdirectories,
+# especially the paragraph on install.libs.R
 dest <- file.path(R_PACKAGE_DIR, paste0("libs", R_ARCH), fsep = "/")
 dir.create(dest, recursive = TRUE, showWarnings = FALSE)
 if (file.exists(src)) {
   print(paste0("Found library file: ", src, " to move to ", dest))
   file.copy(src, dest, overwrite = TRUE)
+
+  symbols_file <- file.path(source_dir, "symbols.rds")
+  if (file.exists(symbols_file)) {
+    file.copy(symbols_file, dest, overwrite = TRUE)
+  }
+
 } else {
   stop(paste0("Cannot find lib_lightgbm", SHLIB_EXT))
 }

include/LightGBM/tree.h

@@ -257,13 +257,11 @@ class Tree {
 
   inline int NumericalDecision(double fval, int node) const {
     uint8_t missing_type = GetMissingType(decision_type_[node]);
-    if (std::isnan(fval)) {
-      if (missing_type != 2) {
-        fval = 0.0f;
-      }
+    if (std::isnan(fval) && missing_type != MissingType::NaN) {
+      fval = 0.0f;
     }
-    if ((missing_type == 1 && IsZero(fval))
-        || (missing_type == 2 && std::isnan(fval))) {
+    if ((missing_type == MissingType::Zero && IsZero(fval))
+        || (missing_type == MissingType::NaN && std::isnan(fval))) {
       if (GetDecisionType(decision_type_[node], kDefaultLeftMask)) {
         return left_child_[node];
       } else {
@@ -279,8 +277,8 @@ class Tree {
 
   inline int NumericalDecisionInner(uint32_t fval, int node, uint32_t default_bin, uint32_t max_bin) const {
     uint8_t missing_type = GetMissingType(decision_type_[node]);
-    if ((missing_type == 1 && fval == default_bin)
-        || (missing_type == 2 && fval == max_bin)) {
+    if ((missing_type == MissingType::Zero && fval == default_bin)
+        || (missing_type == MissingType::NaN && fval == max_bin)) {
       if (GetDecisionType(decision_type_[node], kDefaultLeftMask)) {
         return left_child_[node];
       } else {
@@ -301,7 +299,7 @@ class Tree {
       return right_child_[node];;
     } else if (std::isnan(fval)) {
       // NaN is always in the right
-      if (missing_type == 2) {
+      if (missing_type == MissingType::NaN) {
         return right_child_[node];
       }
       int_fval = 0;

src/io/json11.cpp

@@ -20,13 +20,12 @@
  */
 #include <LightGBM/utils/json11.h>
 
-#include <limits>
-#ifndef LGB_R_BUILD
-  #include <cassert>
-#endif
+#include <LightGBM/utils/log.h>
+
 #include <cmath>
 #include <cstdio>
 #include <cstdlib>
+#include <limits>
 
 namespace json11 {
 
@@ -39,6 +38,8 @@ using std::make_shared;
 using std::initializer_list;
 using std::move;
 
+using LightGBM::Log;
+
 /* Helper for representing null - just a do-nothing struct, plus comparison
  * operators so the helpers in JsonValue work. We can't use nullptr_t because
  * it may not be orderable.
@@ -626,9 +627,7 @@ struct JsonParser final {
      * the input and return res. If not, flag an error.
      */
     Json expect(const string &expected, Json res) {
-        #ifndef LGB_R_BUILD
-          assert(i != 0);
-        #endif
+        CHECK_NE(i, 0)
         i--;
         if (str.compare(i, expected.length(), expected) == 0) {
             i += expected.length();