[c++] remove uses of '..' in headers #6409

jameslamb · 2024-04-08T02:37:05Z

Contributes to #5957
Contributes to #6261
Contributes to #6379

Some of the project's headers use relative paths:

LightGBM/include/LightGBM/utils/common.h

Lines 33 to 34 in 28536a0

    
           #include "../../../external_libs/fast_double_parser/include/fast_double_parser.h" 
        
           #include "../../../external_libs/fmt/include/fmt/format.h"

This causes problems for anyone wanting to use and install lib_lightgbm.so + LightGBM headers to build their own program. Like this:

In file included from /tmp/lightgbm/include/LightGBM/config.h:21:
/tmp/lightgbm/include/LightGBM/utils/common.h:33:10: fatal error: '../../../external_libs/fast_double_parser/include/fast_double_parser.h' file not found
#include "../../../external_libs/fast_double_parser/include/fast_double_parser.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

As of the changes on this branch, make install or similar with LightGBM will install the headers with the following layout:

-- Install configuration: ""
-- Installing: /tmp/lightgbm/bin/lightgbm
-- Installing: /tmp/lightgbm/lib/lib_lightgbm.dylib
-- Installing: /tmp/lightgbm/include/LightGBM
-- Installing: /tmp/lightgbm/include/LightGBM/objective_function.h
-- Installing: /tmp/lightgbm/include/LightGBM/prediction_early_stop.h
-- Installing: /tmp/lightgbm/include/LightGBM/export.h
-- Installing: /tmp/lightgbm/include/LightGBM/config.h
-- Installing: /tmp/lightgbm/include/LightGBM/metric.h
-- Installing: /tmp/lightgbm/include/LightGBM/c_api.h
-- Installing: /tmp/lightgbm/include/LightGBM/application.h
-- Installing: /tmp/lightgbm/include/LightGBM/arrow.tpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_metadata.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_split_info.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_utils.hu
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/vector_cudahost.h
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_algorithms.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_objective_function.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_metric.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_random.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_column_data.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_tree.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_row_data.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/tree.h
-- Installing: /tmp/lightgbm/include/LightGBM/dataset_loader.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils
-- Installing: /tmp/lightgbm/include/LightGBM/utils/chunked_array.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/array_args.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/file_io.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/threading.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/pipeline_reader.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/openmp_wrapper.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/json11.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/common.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/binary_writer.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/log.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/text_reader.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc/yamc_shared_lock.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc/alternate_shared_mutex.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc/yamc_rwlock_sched.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/byte_buffer.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/random.h
-- Installing: /tmp/lightgbm/include/LightGBM/dataset.h
-- Installing: /tmp/lightgbm/include/LightGBM/train_share_states.h
-- Installing: /tmp/lightgbm/include/LightGBM/network.h
-- Installing: /tmp/lightgbm/include/LightGBM/tree_learner.h
-- Installing: /tmp/lightgbm/include/LightGBM/feature_group.h
-- Installing: /tmp/lightgbm/include/LightGBM/meta.h
-- Installing: /tmp/lightgbm/include/LightGBM/arrow.h
-- Installing: /tmp/lightgbm/include/LightGBM/boosting.h
-- Installing: /tmp/lightgbm/include/LightGBM/sample_strategy.h
-- Installing: /tmp/lightgbm/include/LightGBM/bin.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fast_double_parser.h
-- Up-to-date: /tmp/lightgbm/include/LightGBM/utils
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/ostream.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/format-inl.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/ranges.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/xchar.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/core.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/chrono.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/os.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/color.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/args.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/printf.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/compile.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/format.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/std.h

How I tested this

Compiled a small test C++ program interacting with LightGBM through its C API.

details (click me)

On my mac, tried building the library and installing it to /tmp/lightgbm, like this:

cmake \
    -B build \
    -S . \
    -DUSE_OPENMP=OFF \
    -DCMAKE_INSTALL_PREFIX=/tmp/lightgbm

cmake --build build --target install -j4

Compilation succeeded (in this case, using AppleClang 15) and the library and its headers were installed to /tmp/lightgbm.

[  2%] Building CXX object CMakeFiles/lightgbm_capi_objs.dir/src/c_api.cpp.o
...
[ 97%] Linking CXX shared library /Users/jlamb/repos/LightGBM/lib_lightgbm.dylib
[ 97%] Built target _lightgbm
[100%] Linking CXX executable /Users/jlamb/repos/LightGBM/lightgbm
[100%] Built target lightgbm
Install the project...
-- Install configuration: ""
-- Installing: /tmp/lightgbm/bin/lightgbm
-- Installing: /tmp/lightgbm/lib/lib_lightgbm.dylib
-- Installing: /tmp/lightgbm/include/LightGBM
-- Installing: /tmp/lightgbm/include/LightGBM/objective_function.h
-- Installing: /tmp/lightgbm/include/LightGBM/prediction_early_stop.h
-- Installing: /tmp/lightgbm/include/LightGBM/export.h
-- Installing: /tmp/lightgbm/include/LightGBM/config.h
-- Installing: /tmp/lightgbm/include/LightGBM/metric.h
-- Installing: /tmp/lightgbm/include/LightGBM/c_api.h
-- Installing: /tmp/lightgbm/include/LightGBM/application.h
-- Installing: /tmp/lightgbm/include/LightGBM/arrow.tpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_metadata.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_split_info.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_utils.hu
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/vector_cudahost.h
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_algorithms.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_objective_function.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_metric.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_random.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_column_data.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_tree.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/cuda/cuda_row_data.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/tree.h
-- Installing: /tmp/lightgbm/include/LightGBM/dataset_loader.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils
-- Installing: /tmp/lightgbm/include/LightGBM/utils/chunked_array.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/array_args.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/file_io.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/threading.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/pipeline_reader.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/openmp_wrapper.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/json11.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/common.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/binary_writer.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/log.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/text_reader.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc/yamc_shared_lock.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc/alternate_shared_mutex.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/yamc/yamc_rwlock_sched.hpp
-- Installing: /tmp/lightgbm/include/LightGBM/utils/byte_buffer.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/random.h
-- Installing: /tmp/lightgbm/include/LightGBM/dataset.h
-- Installing: /tmp/lightgbm/include/LightGBM/train_share_states.h
-- Installing: /tmp/lightgbm/include/LightGBM/network.h
-- Installing: /tmp/lightgbm/include/LightGBM/tree_learner.h
-- Installing: /tmp/lightgbm/include/LightGBM/feature_group.h
-- Installing: /tmp/lightgbm/include/LightGBM/meta.h
-- Installing: /tmp/lightgbm/include/LightGBM/arrow.h
-- Installing: /tmp/lightgbm/include/LightGBM/boosting.h
-- Installing: /tmp/lightgbm/include/LightGBM/sample_strategy.h
-- Installing: /tmp/lightgbm/include/LightGBM/bin.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fast_double_parser.h
-- Up-to-date: /tmp/lightgbm/include/LightGBM/utils
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/ostream.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/format-inl.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/ranges.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/xchar.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/core.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/chrono.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/os.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/color.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/args.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/printf.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/compile.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/format.h
-- Installing: /tmp/lightgbm/include/LightGBM/utils/fmt/std.h

Ideally, a program wanting to use lib_lightgbm.so should be able to just include LightGBM/c_api.h to link to it. I tried with such a program, like this:

// main.cpp
#include <string>

#include <LightGBM/c_api.h>
#include <LightGBM/dataset.h>

int main(int argc, char *argv[])
{
    const char* config = "boosting=gbdt objective=regression num_iterations=5 verbose=1";

    // hard-coding a path assuming this will be run from the root of the LightGBM repo, just for simplicity
    std::string train_data_file("examples/regression/regression.train");

    BoosterHandle booster_handle = nullptr;
    DatasetHandle dataset_handle = nullptr;

    LGBM_DatasetCreateFromFile(
        train_data_file.c_str(),
        config,
        nullptr,
        &dataset_handle
    );

    LGBM_BoosterCreate(
        dataset_handle,
        config,
        &booster_handle
    );

    // train for 5 iterations
    int is_finished = 0;
    for(int iter=1;iter<5;iter++){
        LGBM_BoosterUpdateOneIter(
            booster_handle,
            &is_finished
        );
    }

    // write model file
    std::string model_file("model_exmple.txt");
    LGBM_BoosterSaveModel(
        booster_handle,
        0, // start_iteration
        -1, // num_iteration
        C_API_FEATURE_IMPORTANCE_SPLIT,
        model_file.c_str()
    );
}

Attempted to compile it like this:

clang++ \
    -std=c++11 \
    -o ./lgb-train \
    -I/tmp/lightgbm/include \
    -l_lightgbm \
    -L/tmp/lightgbm/lib \
    -Wl,-rpath,/tmp/lightgbm/lib \
    ./main.cpp

That fails like this:

In file included from /tmp/lightgbm/include/LightGBM/config.h:21:
/tmp/lightgbm/include/LightGBM/utils/common.h:33:10: fatal error: '../../../external_libs/fast_double_parser/include/fast_double_parser.h' file not found
#include "../../../external_libs/fast_double_parser/include/fast_double_parser.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

On this branch, it succeeds, and the program runs successfully 😁

./lgb-train

[LightGBM] [Info] Loading initial scores...
[LightGBM] [Info] Construct bin mappers from text data time 0.03 seconds
[LightGBM] [Info] Auto-choosing row-wise multi-threading, the overhead of testing was 0.000382 seconds.
You can set `force_row_wise=true` to remove the overhead.
And if memory is not enough, you can set `force_col_wise=true`.
[LightGBM] [Info] Total Bins 6132
[LightGBM] [Info] Number of data points in the train set: 7000, number of used features: 28

Notes for Reviewers

Why only `fast_double_parser` and `fmt`? Isn't this a problem for Boost (`external_libs/compute/`) and Eigen (`external_libs/Eigen`)?

For using LightGBM's C API, only fast_double_parser and fmt headers need to be available to #include <LightGBM/c_api.h>.

…o fix/include-paths

borchero

I have little CMake knowledge but no relative paths makes sense to me 😄

jameslamb · 2024-05-01T17:58:04Z

I have little CMake knowledge but no relative paths makes sense to me 😄

No problem, thanks for taking a look! I've spent a lot of time over the last 6 months getting more familiar with CMake, hopefully will be able to bring more simplifications like this one here.

To be sure about this, let's get another review from either @guolinke or @shiyu1994 before merging this.

guolinke

Thank you, looks good to me!

jameslamb · 2024-05-04T03:37:16Z

Thank you!

[c++] remove uses of '..' in headers

1f19a75

jameslamb added in progress maintenance labels Apr 8, 2024

jameslamb changed the title ~~[c++] remove uses of '..' in headers~~ WIP: [c++] remove uses of '..' in headers Apr 8, 2024

jameslamb added 6 commits April 7, 2024 21:37

update install path

b807bdb

Merge branch 'master' into fix/include-paths

c807cd8

Merge branch 'master' into fix/include-paths

7613ece

Merge branch 'master' into fix/include-paths

a4ebbca

remove build-cran-packageg logic

02ae459

Merge branch 'fix/include-paths' of github.com:microsoft/LightGBM int…

0ca7668

…o fix/include-paths

jameslamb added awaiting review and removed in progress labels May 1, 2024

jameslamb changed the title ~~WIP: [c++] remove uses of '..' in headers~~ [c++] remove uses of '..' in headers May 1, 2024

jameslamb marked this pull request as ready for review May 1, 2024 04:09

jameslamb requested review from guolinke, shiyu1994, jmoralez and borchero as code owners May 1, 2024 04:09

jameslamb mentioned this pull request May 1, 2024

release v4.4.0 #6439

Merged

33 tasks

borchero approved these changes May 1, 2024

View reviewed changes

Merge branch 'master' into fix/include-paths

7edba08

Merge branch 'master' into fix/include-paths

6219958

guolinke approved these changes May 4, 2024

View reviewed changes

jameslamb merged commit 6e78e69 into master May 4, 2024
38 checks passed

jameslamb deleted the fix/include-paths branch May 4, 2024 03:37

jameslamb removed the awaiting review label May 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[c++] remove uses of '..' in headers #6409

[c++] remove uses of '..' in headers #6409

jameslamb commented Apr 8, 2024 •

edited

Loading

borchero left a comment

jameslamb commented May 1, 2024

guolinke left a comment

jameslamb commented May 4, 2024

	#include "../../../external_libs/fast_double_parser/include/fast_double_parser.h"
	#include "../../../external_libs/fmt/include/fmt/format.h"

[c++] remove uses of '..' in headers #6409

[c++] remove uses of '..' in headers #6409

Conversation

jameslamb commented Apr 8, 2024 • edited Loading

How I tested this

Notes for Reviewers

Why only fast_double_parser and fmt? Isn't this a problem for Boost (external_libs/compute/) and Eigen (external_libs/Eigen)?

borchero left a comment

Choose a reason for hiding this comment

jameslamb commented May 1, 2024

guolinke left a comment

Choose a reason for hiding this comment

jameslamb commented May 4, 2024

jameslamb commented Apr 8, 2024 •

edited

Loading

Why only `fast_double_parser` and `fmt`? Isn't this a problem for Boost (`external_libs/compute/`) and Eigen (`external_libs/Eigen`)?