guitargeek / XGBoost-FastForest

Minimal library code to deploy XGBoost models in C++.
MIT License
89 stars 30 forks source link

C++ 98 Compatibility/Feasibility #17

Closed tesriram closed 2 years ago

tesriram commented 2 years ago

Hello Jonas,

We are attempting to back-port your Fast forest code to run on C++ 98. I am strictly a Python developer but am looking to hand this task off to a C++ developer. Do you believe this is a feasible task and if so, how long do you think such a back-port would take. Thank you for your help and input.

guitargeek commented 2 years ago

Hi @tesriram, sorry for the late reply!

Yes, it's a feasible task to port this to C++98. It can probably done in a few hours max.

Is this still relevant for you? Then I can help you with this.

tesriram commented 2 years ago

Hi Jonas,

Thanks for the reply. This task is still relevant to me and I would very much appreciate your help. Some other C++ developers I asked said it could take them days, if not weeks. Please let me know what I can do to help complete this task. Thank you again for your help.

guitargeek commented 2 years ago

Alright, then I think there is a misunderstanding here what it means for the code to "run on C++ 98". I was thinking of only the interface, which you can see is rather slim and only one file: https://github.com/guitargeek/XGBoost-FastForest/blob/master/include/fastforest.h

I think not many changes to the interface are needed to make fastforest.h includable in a C++98 project.

If you meant that the fastforest library should compile itself with C++98, you need to migrate everything in src as well, which takes a much longer time indeed (in C++98 you can't even use the standard library). I think before investing the time to do this, we need to understand whether it's necessary that fastforest compiles with C++98, or whether migrating only the interface is enough.

Can you not compile fastforest with a modern compiler, and then use the library in your project that sticks to the C++98 standard, assuming that fastforest.h would also be made C++98 conform?

guitargeek commented 2 years ago

I have just pushed the changes to fastforest.h to make it C++98 conform.

Here a code snippet to test (complie with g++ -std=c++98 -pedantic -o test98 test98.cpp -lfastforest):

// test98.cpp

#include <fastforest.h>

#include <iostream>
#include <fstream>

int main() {

    std::cout << "Hello World 98" << std::endl;

    std::vector<std::string> features;
    features.push_back("f0");
    features.push_back("f1");
    features.push_back("f2");
    features.push_back("f3");
    features.push_back("f4");

    fastforest::FastForest fastForest = fastforest::load_txt("continuous/model.txt", features);

    std::ifstream fileX("continuous/X.csv");
    std::ifstream filePreds("continuous/preds.csv");

    std::vector<fastforest::FeatureType> input(5);
    fastforest::FeatureType score;
    float ref;
    std::size_t nSamples = 100;

    for (std::size_t i = 0; i < nSamples; ++i) {
        for (std::size_t j = 0; j < input.size(); ++j) {
            fileX >> input[j];
        }
        score = fastForest(input.data());
        filePreds >> ref;
        std::cout << score << "   " << ref << std::endl;
    }

    return 0;

}

So you can use the library with C++98. Is that enough for you, or do you really need to build it with a C++ compiler that supports only C++98 as well?

tesriram commented 2 years ago

Thank you very much for your suggestions and for writing that code. One of the C++ developers for the system we are trying to integrate with said that we need the interface and the library to be compiled using gcc -std=c++98 compiler flag. He mentioned that there might be a 'transpiler' that translates code to an older version, but if not, then the whole library would need to be backported. If this is the case, approximately how long do you think it would take to backport the whole library?

guitargeek commented 2 years ago

Hi, okay thanks for clarifying! A transpiler is not necessary here I think, as the library is small and the whole code can be adapted by hand to not use features from standards newer than C++98. I have done this in the most recent commit, now the library will compile also with the -std=c++98 compiler flag (at least if you don't use the EXPERIMENTAL_TMVA_SUPPORT, which I doubt you do).

Maybe that works for you already? Unfortunately the tests can't be compiled with C++98 yet, is that a problem? If yes, I think they can be easily migrated too.

How much time you still need for the backport to your particular environment I don't know. Could be anything between zero and multiple days if you use an old gcc that still has some issues that affect this library, and you need to work around them.

Which gcc version are you using by the way? I would like for the continuous integration to also test the library with an older gcc, but I need some input for picking a version :)

guitargeek commented 2 years ago

With the most recent commits, I have adapted all the code to build with C++98, including the TMVA support and the tests.

I'll therefore close this issue, feel free to open another issue if you have further problems!

Cheers, Jonas