Cannot compile during build due to POMDP/IO.cpp

[BenjaminSchlaich]

Hello!
I managed to get to the build phase on MacOS, but when executing "make" I encounter the following error:

(just updated from copy&pasting the log to a screenshot, which is far more readable here)

I suppose that this is a problem with my compiler version?
Would you recommend further investigating this, or should I just use the Dockerfile from #69?
The problem is, I have a rather old device and Docker doesn't run quite smoothly on it.
Thank you for any help! Best regards, Benjamin.

[BenjaminSchlaich]

Nevermind, I found out that this is actually a problem with clang. Namely, apple clang (in contrast to regular clang, gcc and msvc) does not support the C++20 feature "Parenthesized initialization of aggregates", as can be seen here: https://en.cppreference.com/w/cpp/compiler_support/20.
This now leaves Mac users with 3 options, the way I see it:

1. Install regular clang using homebrew, as explained here: https://stackoverflow.com/questions/64225495/how-to-switch-from-apple-to-homebrew-installed-clang
2. Apply minimal changes to the code everywhere that this syntax is used.
3. Use a docker container for Linux and avoid apple clang entirely.

Option 3 is discussed here already and I have nothing to say about it since I didn't try it: #69
I also ruled out option 1, since I really don't feel like messing around with my clang installation and getting into trouble with compiler paths.
I went with Option 2 and managed to make the whole library build all targets successfully within an hour.
Everywhere where the make script complained I went to the code and made a change like for example so:

// this is the original code
predictors_.emplace_back(entropyBins * scaling, ubBins * scaling, initialUbQ);

// this is the apple clang compatible version without in-place construction
predictors_.emplace_back((LBPredictor) {entropyBins * scaling, ubBins * scaling, initialUbQ});

There are 11 places in the code files where we have to apply this kind of change, so it won't take forever with some clever regex's.
Two drawbacks from this way of solving the issue are obvious to me:

1. There might be performance loss, because the vector elements aren't constructed in-place anymore.
2. This is a temporary fix to get the library to work at all, because with every update I would have to do it all over!

Do you think that there will be any chance that the code will be compatible with apple clang? I would be happy to help and share my edited code files, if that is of any use to you. However, I am not sure how serious the performance drawback really is, so maybe this doesn't make sense at all. Please feel free to let me know if you know of some easier fix for this.
Best regards, Benjamin

[Svalorzen]

Hey, thanks for the detailed report. I think it's unlikely that there are going to be significant performance losses from your changes, so I wouldn't worry about it.

However, in terms of backward support, I'm afraid I'm going to have to disappoint you. This project at the moment is as much as an independent algorithm repository as it is a personal project of mine. For as many people that have used it over the years, I've never really received significant contributions, and so I'm generally keeping the code in a state that is easier for me to maintain rather than the opposite.

Technically the project wouldn't even require C++20; there's nothing specific that couldn't be done with an earlier C++ version (and indeed when the project was first started it was in C++11). Still, over time I simplified things using new features as it made improving the whole library as a whole easier for me. As harsh as it may sound, there's really very little incentive for me to go back and support your compiler version -- after that, what if somebody with an even earlier compiler wants support for something else? Where would it stop? As you saw with your earlier issue, I barely have the resources to (badly) support different platforms. Instead, what I've done over the years is to move features forward as they come along, both for the library and also as a testbed for me to learn and try new C++ features.

If it can be of any reassurance, at the moment I don't really have a lot of time to update the library, so it is unlikely to change significantly in the near future, so you probably won't have to reapply those fixes very often. And who knows, by the time I add something else, perhaps apple clang will have improved and added support for that type of initialization.

Sorry to disappoint, but I don't see this changing any time soon.

[BenjaminSchlaich]

Hey! Thanks for the quick response, again! I can perfectly understand that you do not have the resources to go and make your code compatible with any specific compiler version. Honestly, if I have any disappointment, it is because apple clang won't adopt this kind of initialization when all other major C++ compilers already do. This really enforces those kind of compatibility issues in my opinion.
I am very grateful for your work! So far this library seems to me to be a real treasure!
I would be glad to provide my services to you and help with the development, but unfortunately I have very little knowledge of reinforcement learning so far, and want to use the library precisely to learn more about it.

As you already mentioned, I believe that in the long run this problem will resolve itself because apple clang will eventually implement parenthesized initialization of aggregates. And until then I do have a fix to make the library work, so I'll close this as resolved.