From 1f45ab45f7f0bcf317664c3d1a84e9cb367c498a Mon Sep 17 00:00:00 2001 From: Drew Miller Date: Tue, 19 Oct 2021 10:39:43 +0100 Subject: [PATCH 1/5] [c_api] Improve ANSI compatibility by avoiding --- include/LightGBM/c_api.h | 11 ++++++++--- src/c_api.cpp | 8 ++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/include/LightGBM/c_api.h b/include/LightGBM/c_api.h index b5542891b063..2cd949563029 100644 --- a/include/LightGBM/c_api.h +++ b/include/LightGBM/c_api.h @@ -23,7 +23,6 @@ #include #include #include -#include #endif @@ -434,12 +433,12 @@ LIGHTGBM_C_EXPORT int LGBM_DatasetAddFeaturesFrom(DatasetHandle target, /* --- start Booster interfaces */ /*! -* \brief Get boolean representing whether booster is fitting linear trees. +* \brief Get int representing whether booster is fitting linear trees. * \param handle Handle of booster * \param[out] out The address to hold linear trees indicator * \return 0 when succeed, -1 when failure happens */ -LIGHTGBM_C_EXPORT int LGBM_BoosterGetLinear(BoosterHandle handle, bool* out); +LIGHTGBM_C_EXPORT int LGBM_BoosterGetLinear(BoosterHandle handle, int* out); /*! * \brief Create a new boosting learner. @@ -1355,11 +1354,17 @@ static char* LastErrorMsg() { static THREAD_LOCAL char err_msg[512] = "Everythin #endif /*! * \brief Set string message of the last error. + * \note + * This will call unsafe ``sprintf`` when compiled using C standards before C99. * \param msg Error message */ INLINE_FUNCTION void LGBM_SetLastError(const char* msg) { const int err_buf_len = 512; +#if !defined(__cplusplus) && (!defined(__STDC__) || (__STDC_VERSION__ < 199901L)) + sprintf(LastErrorMsg(), "%s", msg); +#else snprintf(LastErrorMsg(), err_buf_len, "%s", msg); +#endif } #endif /* LIGHTGBM_C_API_H_ */ diff --git a/src/c_api.cpp b/src/c_api.cpp index bc3bfc3b2434..9c6c903d5da8 100644 --- a/src/c_api.cpp +++ b/src/c_api.cpp @@ -1657,10 +1657,14 @@ int LGBM_BoosterGetNumClasses(BoosterHandle handle, int* out_len) { API_END(); } -int LGBM_BoosterGetLinear(BoosterHandle handle, bool* out) { +int LGBM_BoosterGetLinear(BoosterHandle handle, int* out) { API_BEGIN(); Booster* ref_booster = reinterpret_cast(handle); - *out = ref_booster->GetBoosting()->IsLinear(); + if(ref_booster->GetBoosting()->IsLinear()) { + *out = 1; + } else { + *out = 0; + } API_END(); } From cce047e0ed9edb07d648eb2c164e76877789b4aa Mon Sep 17 00:00:00 2001 From: Drew Miller Date: Tue, 19 Oct 2021 12:19:47 +0100 Subject: [PATCH 2/5] fixes in response to CI linting --- .ci/test.sh | 3 ++- src/c_api.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.ci/test.sh b/.ci/test.sh index 047395797951..f3c500dd42c1 100755 --- a/.ci/test.sh +++ b/.ci/test.sh @@ -80,7 +80,8 @@ if [[ $TASK == "lint" ]]; then echo "Linting R code" Rscript ${BUILD_DIRECTORY}/.ci/lint_r_code.R ${BUILD_DIRECTORY} || exit -1 echo "Linting C++ code" - cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive ./src ./include ./R-package ./swig ./tests || exit -1 + cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length,-runtime/printf ./include/LightGBM/c_api.h || exit -1 + cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive --exclude=./include/LightGBM/c_api.h ./src ./include ./R-package ./swig ./tests || exit -1 cmake_files=$(find . -name CMakeLists.txt -o -path "*/cmake/*.cmake") cmakelint --linelength=120 ${cmake_files} || true exit 0 diff --git a/src/c_api.cpp b/src/c_api.cpp index 9c6c903d5da8..c9972edfa4d7 100644 --- a/src/c_api.cpp +++ b/src/c_api.cpp @@ -1660,7 +1660,7 @@ int LGBM_BoosterGetNumClasses(BoosterHandle handle, int* out_len) { int LGBM_BoosterGetLinear(BoosterHandle handle, int* out) { API_BEGIN(); Booster* ref_booster = reinterpret_cast(handle); - if(ref_booster->GetBoosting()->IsLinear()) { + if (ref_booster->GetBoosting()->IsLinear()) { *out = 1; } else { *out = 0; From 8c2c1bb632055dac54e17d19d589caf187c456db Mon Sep 17 00:00:00 2001 From: Drew Miller Date: Wed, 20 Oct 2021 09:50:27 +0100 Subject: [PATCH 3/5] inline NOLINT instead of separate test --- .ci/test.sh | 3 +-- include/LightGBM/c_api.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.ci/test.sh b/.ci/test.sh index f3c500dd42c1..047395797951 100755 --- a/.ci/test.sh +++ b/.ci/test.sh @@ -80,8 +80,7 @@ if [[ $TASK == "lint" ]]; then echo "Linting R code" Rscript ${BUILD_DIRECTORY}/.ci/lint_r_code.R ${BUILD_DIRECTORY} || exit -1 echo "Linting C++ code" - cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length,-runtime/printf ./include/LightGBM/c_api.h || exit -1 - cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive --exclude=./include/LightGBM/c_api.h ./src ./include ./R-package ./swig ./tests || exit -1 + cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive ./src ./include ./R-package ./swig ./tests || exit -1 cmake_files=$(find . -name CMakeLists.txt -o -path "*/cmake/*.cmake") cmakelint --linelength=120 ${cmake_files} || true exit 0 diff --git a/include/LightGBM/c_api.h b/include/LightGBM/c_api.h index 2cd949563029..0d7fe9319968 100644 --- a/include/LightGBM/c_api.h +++ b/include/LightGBM/c_api.h @@ -1361,7 +1361,7 @@ static char* LastErrorMsg() { static THREAD_LOCAL char err_msg[512] = "Everythin INLINE_FUNCTION void LGBM_SetLastError(const char* msg) { const int err_buf_len = 512; #if !defined(__cplusplus) && (!defined(__STDC__) || (__STDC_VERSION__ < 199901L)) - sprintf(LastErrorMsg(), "%s", msg); + sprintf(LastErrorMsg(), "%s", msg); /* NOLINT(runtime/printf) */ #else snprintf(LastErrorMsg(), err_buf_len, "%s", msg); #endif From d6242412454910fb98d11a63a340129d9fb8a262 Mon Sep 17 00:00:00 2001 From: Drew Miller Date: Fri, 22 Oct 2021 08:45:41 +0100 Subject: [PATCH 4/5] moving length declaration to non-ANSI C conditional --- include/LightGBM/c_api.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/LightGBM/c_api.h b/include/LightGBM/c_api.h index 284a91467e7b..debe163359ae 100644 --- a/include/LightGBM/c_api.h +++ b/include/LightGBM/c_api.h @@ -1365,10 +1365,10 @@ static char* LastErrorMsg() { static THREAD_LOCAL char err_msg[512] = "Everythin * \param msg Error message */ INLINE_FUNCTION void LGBM_SetLastError(const char* msg) { - const int err_buf_len = 512; #if !defined(__cplusplus) && (!defined(__STDC__) || (__STDC_VERSION__ < 199901L)) sprintf(LastErrorMsg(), "%s", msg); /* NOLINT(runtime/printf) */ #else + const int err_buf_len = 512; snprintf(LastErrorMsg(), err_buf_len, "%s", msg); #endif } From fa6e921e9244f453398ab22237fdbf04d6a51722 Mon Sep 17 00:00:00 2001 From: Drew Miller Date: Sat, 13 Nov 2021 14:26:07 +0000 Subject: [PATCH 5/5] [c_api] Align expected return type in `basic.py` with new c_api type. --- python-package/lightgbm/basic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python-package/lightgbm/basic.py b/python-package/lightgbm/basic.py index 83a4b5c071da..a44e5831f9c7 100644 --- a/python-package/lightgbm/basic.py +++ b/python-package/lightgbm/basic.py @@ -3566,7 +3566,7 @@ def refit(self, data, label, decay_rate=0.9, **kwargs): predictor = self._to_predictor(deepcopy(kwargs)) leaf_preds = predictor.predict(data, -1, pred_leaf=True) nrow, ncol = leaf_preds.shape - out_is_linear = ctypes.c_bool(False) + out_is_linear = ctypes.c_int(0) _safe_call(_LIB.LGBM_BoosterGetLinear( self.handle, ctypes.byref(out_is_linear))) @@ -3575,7 +3575,7 @@ def refit(self, data, label, decay_rate=0.9, **kwargs): params=self.params, default_value=None ) - new_params["linear_tree"] = out_is_linear.value + new_params["linear_tree"] = bool(out_is_linear.value) train_set = Dataset(data, label, silent=True, params=new_params) new_params['refit_decay_rate'] = decay_rate new_booster = Booster(new_params, train_set)