code refactoring: cost effective gradient boosting

[guolinke]

refer to #2341

[guolinke]

@StrikerRUS any style issues?

[StrikerRUS]

@guolinke

any style issues?

I'll run cpplint on this branch tomorrow.

[guolinke]

@StrikerRUS maybe we can add cpplint into CI, but don't throw the error? (or ignore some types?)

[StrikerRUS]

@guolinke

maybe we can add cpplint into CI, but don't throw the error? (or ignore some types?)

I like this idea! I'll create a PR for that.

any style issues?

Here is a part of report from cpplint related to files with diff:

./src\treelearner\serial_tree_learner.h:5:  #ifndef header guard has wrong style, please use: SRC_TREELEARNER_SERIAL_TREE_LEARNER_H_  [build/header_guard] [5]
./src\treelearner\serial_tree_learner.h:195:  #endif line should be "#endif  // SRC_TREELEARNER_SERIAL_TREE_LEARNER_H_"  [build/header_guard] [5]
./src\treelearner\serial_tree_learner.h:32:  Do not use namespace using-directives.  Use using-declarations instead.  [build/namespaces] [5]
./src\treelearner\serial_tree_learner.h:83:  Do not leave a blank line after "protected:"  [whitespace/blank_line] [3]
./src\treelearner\serial_tree_learner.h:111:  Is this a non-const reference? If so, make const or use a pointer: Json& forced_split_json  [runtime/references] [2]
Done processing ./src\treelearner\serial_tree_learner.h

./src\treelearner\serial_tree_learner.cpp:5:  Include the directory when naming .h files  [build/include_subdir] [4]
./src\treelearner\serial_tree_learner.cpp:8:  Found C system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:9:  Found C system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:10:  Found C system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:11:  Found C system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:13:  Found C++ system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:14:  Found C++ system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:15:  Found C++ system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:16:  Found C++ system header after other header. Should be: serial_tree_learner.h, c system, c++ system, other.  [build/include_order] [4]
./src\treelearner\serial_tree_learner.cpp:288:  Consider using CHECK_GE instead of CHECK(a >= b)  [readability/check] [2]
./src\treelearner\serial_tree_learner.cpp:291:  Missing space before ( in if(  [whitespace/parens] [5]
./src\treelearner\serial_tree_learner.cpp:300:  Consider using CHECK_GE instead of CHECK(a >= b)  [readability/check] [2]
./src\treelearner\serial_tree_learner.cpp:312:  Consider using CHECK_GE instead of CHECK(a >= b)  [readability/check] [2]
./src\treelearner\serial_tree_learner.cpp:874:  Consider using CHECK_GT instead of CHECK(a > b)  [readability/check] [2]
Done processing ./src\treelearner\serial_tree_learner.cpp

./src\treelearner\cost_effective_gradient_boosting.hpp:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
./src\treelearner\cost_effective_gradient_boosting.hpp:0:  No #ifndef header guard found, suggested CPP variable is: SRC_TREELEARNER_COST_EFFECTIVE_GRADIENT_BOOSTING_HPP_  [build/header_guard] [5]
./src\treelearner\cost_effective_gradient_boosting.hpp:7:  Include the directory when naming .h files  [build/include_subdir] [4]
./src\treelearner\cost_effective_gradient_boosting.hpp:13:  public: should be indented +1 space inside class CostEfficientGradientBoosting  [whitespace/indent] [3]
./src\treelearner\cost_effective_gradient_boosting.hpp:14:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [5]
./src\treelearner\cost_effective_gradient_boosting.hpp:15:  Redundant blank line at the start of a code block should be deleted.  [whitespace/blank_line] [2]
./src\treelearner\cost_effective_gradient_boosting.hpp:15:  Redundant blank line at the end of a code block should be deleted.  [whitespace/blank_line] [3]
./src\treelearner\cost_effective_gradient_boosting.hpp:75:  private: should be indented +1 space inside class CostEfficientGradientBoosting  [whitespace/indent] [3]
./src\treelearner\cost_effective_gradient_boosting.hpp:76:  Do not leave a blank line after "private:"  [whitespace/blank_line] [3]
./src\treelearner\cost_effective_gradient_boosting.hpp:101:  Add #include <vector> for vector<>  [build/include_what_you_use] [4]
Done processing ./src\treelearner\cost_effective_gradient_boosting.hpp

I guess, a fix for all headers order issues will be placing #include "cost_effective_gradient_boosting.hpp" after #include <utility> with one blank line. I remember we have the following order of includes:

dir2/foo2.h (or "foo2.hpp") (it is a header with the same name as example cpp file)
A blank line
Your project's .h files in include folder  (in the alphabetical order).
A blank line
C system files (in the alphabetical order).
C++ system files (in the alphabetical order).
A blank line
Other libraries' .h (or .hpp) files  (in the alphabetical order).
Your project's .h (or .hpp) files in src folder  (in the alphabetical order).

LightGBM/src/io/parser.cpp

Lines 5 to 11 in 0237492

	#include "parser.hpp"
	#include <string>
	#include <fstream>
	#include <functional>
	#include <iostream>
	#include <memory>

LightGBM/src/io/bin.cpp

Lines 5 to 18 in 0237492

	#include <LightGBM/bin.h>
	#include <LightGBM/utils/common.h>
	#include <LightGBM/utils/file_io.h>
	#include <algorithm>
	#include <cmath>
	#include <cstdint>
	#include <cstring>
	#include "dense_bin.hpp"
	#include "dense_nbits_bin.hpp"
	#include "ordered_sparse_bin.hpp"
	#include "sparse_bin.hpp"

[StrikerRUS]

Remaining cpplint errors:

./src\treelearner\cost_effective_gradient_boosting.hpp:16:  Include the directory when naming .h files  [build/include_subdir] [4]
./src\treelearner\cost_effective_gradient_boosting.hpp:82:  private: should be indented +1 space inside class CostEfficientGradientBoosting  [whitespace/indent] [3]
./src\treelearner\cost_effective_gradient_boosting.hpp:82:  "private:" should be preceded by a blank line  [whitespace/blank_line] [3]
./src\treelearner\cost_effective_gradient_boosting.hpp:111:  At least two spaces is best between code and comments  [whitespace/comments] [2]
Done processing ./src\treelearner\cost_effective_gradient_boosting.hpp

./src\treelearner\serial_tree_learner.cpp:5:  Include the directory when naming .h files  [build/include_subdir] [4]
./src\treelearner\serial_tree_learner.cpp:289:  Consider using CHECK_GE instead of CHECK(a >= b)  [readability/check] [2]
./src\treelearner\serial_tree_learner.cpp:292:  Missing space before ( in if(  [whitespace/parens] [5]
./src\treelearner\serial_tree_learner.cpp:301:  Consider using CHECK_GE instead of CHECK(a >= b)  [readability/check] [2]
./src\treelearner\serial_tree_learner.cpp:313:  Consider using CHECK_GE instead of CHECK(a >= b)  [readability/check] [2]
./src\treelearner\serial_tree_learner.cpp:875:  Consider using CHECK_GT instead of CHECK(a > b)  [readability/check] [2]
Done processing ./src\treelearner\serial_tree_learner.cpp

./src\treelearner\serial_tree_learner.h:32:  Do not use namespace using-directives.  Use using-declarations instead.  [build/namespaces] [5]
./src\treelearner\serial_tree_learner.h:83:  Do not leave a blank line after "protected:"  [whitespace/blank_line] [3]
./src\treelearner\serial_tree_learner.h:111:  Is this a non-const reference? If so, make const or use a pointer: Json& forced_split_json  [runtime/references] [2]
Done processing ./src\treelearner\serial_tree_learner.h

[guolinke]

Thanks @StrikerRUS , I am in the mobile device, will fix it tomorrow.

[StrikerRUS]

I guess, https://github.com/microsoft/LightGBM/blob/master/windows/LightGBM.vcxproj and https://github.com/microsoft/LightGBM/blob/master/windows/LightGBM.vcxproj.filters should be updated as well.

[guolinke]

@StrikerRUS any ideas about the python lint error?

[StrikerRUS]

@guolinke Errors are due to the new version of pydocstyle, refer to #2451. I introduced new label blocking, which means that this PR should be merged firstly to pass CI (or maybe another reason).