Roadmap for compiling roadrunner with C++20

[CiaranWelsh]

I've managed to get roadrunner compiled with C++20. Its not currently working in Azure, but this is a complication that will go away if/when we merge into develop branch. (The problem is regarding the main roadrunner pipeline is downloading the wrong dependnecy package from the libroadruner-deps pipeline. Ive tested manually on linux and windows.)

The following has been implemented on branch:
- cxx-20 in roadrunner
- compile-with-cxx-20 in libroadrunner-deps
- cxx-20 in sys-bio/libsbml (not in sbmlteam/libsbml)

Should we merge this into develop?

These are the changes that were made:

1. Fork sbml to the sys-bio repo.
2. change the remote that our sbml submodule points to so that it points to our fork instead of the official sbml repository. This allows us to send a pull request and potentially get any changes we implement integrated into sbml, with ease. This is needed becasuse we need to add a few minor additions to sbml to get it to compile with > C++14
3. update poco. The version we have does not compile with C++17, but its really easy update with git submodules.
4. Unfortunately, having poco as a submodule adds a little extra complexity, but nothing we can't deal with:
   - both sbml and poco define an "uninstall" target, and we can't have both so one of them needs to change. The obvious choice is poco since it's less likely we'll want to update it again. We just need to change the name of the uninstall target to uninstall_poco.
   - The new poco has messed up the location of its "config" files (aka PocoConfig.cmake). Therefore, we add this new location to our CMAKE_PREFIX_PATH. The other option is to modify where poco installs these config files, but its easier to just accept it.
5. Update SBML
   - SBML uses some deprecated std library functions, std::unary_function and std::binary_function. The easiest solution (though not best) is to go and find these templates and include them with sbml source. Then we remove std:: from all uses of std::unary_function or std::binary_function.
   - SBML uses a std::fun_ptr which is also deprecated. So we do the same thing as above: find the original source and include it with sbml.
   - These changes have been made on a new branch in our own sbml fork.
6. The Poco update introduced a few bugs in roadrunner, particurarly with the logger. These are minor and all we need to do to fix them is to change pocoLogger->getChannel() to pocoLogger->getChannel().get().
7. Lastly, some of the windows specific functions used in managing the files have changed string types. Eventually, we should replace all of this with calls to std::filesystem library, but for now there are a few easy fixed to get roadrunner to compile:
   - rrUtils.cpp:
   - line 418, cast char* to LPCTSTR
   - line 711, change existing cast from LPTSTR to LPSTR
   - rrc_api.cpp:
   - line 244 need a reinterpret_cast<LPCWSTR>
   - Suite_TestModel.cpp
   - line 163: need a reinterpret_cast<LPCWSTR>
   - line 167 need a reinterpret_cast<const char*>

[hsauro]

I’ll let you and Lucian decide on a merge.

[luciansmith]

The libsbml issue is being worked on: sbmlteam/libsbml#36 sbmlteam/deviser#51 My inclination is to not fork libsbml, but instead contribute to a branch if we need to, so that branch can be folded into the core more easily. Remember that if we have a problem with libsbml, that means that everyone potentially has the same problem, and it's a community project that we are among the major members of--we should contribute a solution instead of going off on our own.

[CiaranWelsh]

I agree with not going off on our own but not that we shouldn't have our own fork of sbml. Forking, making experimental changes and then sending a pull request is pretty much the standard way for contributing to other projects. On the other hand, I wasn't aware that sbml was autogenerated code. In this case, I'm not confident enough on how to make these fixes anyway.

Its good to see people are already working on this problem, but this has already been an issue for a long time, if we wait it might be months before we can use modern C++ features in roadrunner. Personally, I'd rather put in a placeholder and start developing with newer C++ now. Then, when the official fix is available its easy enough to switch back. The code we need to start doing this is almost ready to go and the only thing we achieve by waiting is not having the shiniest new toys to play with.

That's my vote, but the end decision is yours I think :-)

[luciansmith]

I know creating forks and pushing and pulling all over the place is standard, but I kind of hate it. I feel like it's a hack that sort of works for large cumbersome projects where you can't solve the problem properly. In this case, I'm literally on the SBML team and have write access to the source code; if we have a fix I should create a branch and check it in directly. I'm not saying we should wait for an official fix; I'm saying we should be the official fix. If we can figure out what to do; I've looked into this a couple times myself, but don't have a good solution, either.

(Also, SBML is not autogenerated code, for the most part. Some of the package code is, but not core stuff, and certainly not the code associated with this issue.)

Also, while making roadrunner compilable on c++20 is absolutely a good goal, using c++20 features in the library and thus making it only c++20 compatible is a no-go, right now. We have to be able to compile the thing on manyLinux, which I think is CentOS from 10 years ago or something, not to mention MacOS10.9. We cannot add c++20 features to roadrunner directly any time soon. (Though maybe you want to be able to use the roadrunner library in a project that itself doesn't have to be useful to anyone other than yourself? That would be a fine use of c++20.)

[CiaranWelsh]

Understood. On the sbml, the fix I have is a bit of a hack and really shouldn't be pushed to the main sbml repository. But there weren't that many problems to be honest - maybe naive of me but it doesn't seem that it should take too much effort to implement a proper fix. (Famous last words :D).

On the c++20, I wasn't thinking of doing anything that would prevent backwards compatibility with compilers. My main interest was with std::filesystem for a standard interface to the operating system and and std::execution for easy performance gains - for this we actually only need c++17. Compiling on manylinux and old mac is an angle I didn't consider, but then for this don't we just install a newer compiler? I don't want to argue the point any more, happy to accept the "no" (hence closing this issue), but it seems like with a little effort we could start using some of these newer features (carefully).

[matthiaskoenig]

Just to chip in.
I highly support Lucian here. Please contribute the fixes directly to libsbml. Otherwise there are many downstream issues.
A good example is the fork of libsbml for tellurium (tesbml) which created more issues then solutions in the long run.

[luciansmith]

Just to note: this is now fixed in libsbml by Sarah, so all we now need to do is update our pointers to the libsbml code.