AMICI-dev/AMICI

Don't handle model entity names via preprocessor

dweindl opened this issue · 1 comments

The preprocessor defines we are currently using to have more readable model equations are potentially problematic because they aren't scoped. This can lead to issues like:

In file included from /home/runner/work/AMICI/AMICI/test_bmc/Smith_BMCSystBiol2013/stau.cpp:8:
/home/runner/work/AMICI/AMICI/test_bmc/Smith_BMCSystBiol2013/x.h:1: error: "NULL" redefined [-Werror]
    1 | #define NULL x[0]
      | 
In file included from /usr/include/stdio.h:33,
                 from /usr/include/c++/11/cstdio:42,
                 from /usr/include/c++/11/ext/string_conversions.h:43,
                 from /usr/include/c++/11/bits/basic_string.h:6608,
                 from /usr/include/c++/11/string:55,
                 from /usr/include/c++/11/bits/locale_classes.h:40,
                 from /usr/include/c++/11/bits/ios_base.h:41,
                 from /usr/include/c++/11/ios:42,
                 from /home/runner/.local/lib/python3.9/site-packages/amici/include/gsl/gsl-lite.hpp:26,
                 from /home/runner/work/AMICI/AMICI/test_bmc/Smith_BMCSystBiol2013/stau.cpp:5:
/usr/lib/gcc/x86_64-linux-gnu/11/include/stddef.h:392: note: this is the location of the previous definition
  392 | #define NULL __null
      | 

Would be good to come up with some other solution that allows compile-time mapping for readable names to array locations.

Related to #1544

One alternative could be generating, for example, a class Parameters that wraps the plain parameter array and has as members the names of all parameters that map to the respective array location. So what is currently written as p1 + p2 would become p.p1 + p.p2. The main advantage would be, that we have everything inside the class namespace, so it's safe if somebody wants to use a parameter named p. Slightly more verbose, but on the positive side, it would be obvious that those are parameters. It still wouldn't work for c++ reserved names, so no parameter named int etc. would be supported. If we want that, then I guess there is no way around using a string-indexed map.
Seems to be possible to resolve some string-based mapping at compile time (e.g. https://github.com/serge-sans-paille/frozen/). However, that would make the code less readable, e.g. p["p1"] + p["p2"] (or probably worse, p[_p["p1"]] + p[_p["p2"]], because we need to look up indices, not values, as the parameter values aren't known at compile time).

EDIT: Okay, a simple consteval function for resolving string to index mapping would work without additional run-time cost, but it would require writing something like p[_p("p1")] + p[_p("p2")], or with a bit of macros (and code printing adaptations) PAR("p1") + PAR("p2"), which is actually not that bad. At least we wouldn't have to bother with any reserved names in amici/c++/... . Happy about better solutions.