Wizard recipe: MAiNGO-v0.7.2

[MAiNGO-github]

This pull request contains a new build recipe we built using the BinaryBuilder.jl wizard:

Package name: MAiNGO
Version: v0.7.2

@staticfloat please review and merge.

Auditor complains about AVX2

Without setting march the Auditor detects avx2.
However, after using march="avx2", avx512 is detected.
Thus, we build without march as it is either a false detection or some dependency adapts to the march setting.
In both cases the selected setting should result in the most compatible binaries.

Why only Windows and Linux ?

MacOS support is held back by the following issue (XCode seems to be too old (?)): #7139
Note that we tried the suggestion in JuliaPackaging/BinaryBuilder.jl#1263 locally under Linux, but we received many errors for

rm -rf /opt/${target}/${target}/sys-root/System
cp -raf usr/* "/opt/${target}/${target}/sys-root/usr/."
cp -raf System "/opt/${target}/${target}/sys-root/."

similar to what was described in the issue comments.

FreeBSD was once successfully compiled using -DCMAKE_TOOLCHAIN_FILE=${CMAKE_/TARGET_TOOLCHAIN%.*}_gcc.cmake, but MAiNGO is not properly tested on that platform.

[giordano]

Why? Isn't this already the case?

Suggested change

git remote set-url origin https://git.rwth-aachen.de/avt-svt/public/maingo.git

[Downsite]

I ran into this problem before, at least in my tests locally the git remote is changed for caching.
See JuliaPackaging/BinaryBuilderBase.jl#355

[MAiNGO-github]

Yes, in our tests, this did not work because of caching. For now we would leave this in.

[giordano]

but we received many errors for

rm -rf /opt/${target}/${target}/sys-root/System
cp -raf usr/* "/opt/${target}/${target}/sys-root/usr/."
cp -raf System "/opt/${target}/${target}/sys-root/."

similar to what was described in the issue comments.

You need a privileged runner to be able to delete files from the root file system: https://docs.binarybuilder.org/stable/environment_variables/. That's what we use here in CI, which is what eventually matters for actually building the pacakge.

[fingolfin]

No v3 on purpose?

[MAiNGO-github]

We expand only to gfortan 4 and 5 (3 seems not to have std::variant)

[MAiNGO-github]

@giordano maybe you could take another look.
We followed the suggestions about filtering out platforms instead of picking platforms that work.

Using GCC the build is successful for MacOS, but FreeBSD did not succeed, which would have to be investigated further and fixed upstream.

[giordano]

Why not clang? Also, at this point you could avoid the conditional and use gcc everywhere, but I'd prefer to understand what's wrong with clang, that's usually the best choice for macOS.

[giordano]

The error message on x86_64 macOS is (multiple times)

[13:29:02] In file included from /workspace/srcdir/maingo/dep/libale/src/symbol.cpp:12:
[13:29:02] In file included from /workspace/srcdir/maingo/dep/libale/src/symbol.hpp:14:
[13:29:02] In file included from /workspace/srcdir/maingo/dep/libale/src/value.hpp:16:
[13:29:02] /workspace/srcdir/maingo/dep/libale/src/tensor.hpp:460:31: error: no member named 'to_string' in namespace 'std'
[13:29:02]               "index " + std::to_string(index) + " out of bounds for shape = " + std::to_string(shape(0)));
[13:29:02]                          ~~~~~^
[13:29:02] /workspace/srcdir/maingo/dep/libale/src/tensor.hpp:460:87: error: no member named 'to_string' in namespace 'std'
[13:29:02]               "index " + std::to_string(index) + " out of bounds for shape = " + std::to_string(shape(0)));
[13:29:02]                                                                                  ~~~~~^
[13:29:02] 2 errors generated.

According to pytorch/pytorch#3192 (comment) there may be a missing

#include <string>

Also C++ docs shows that std::to_string requires the string header file: https://en.cppreference.com/w/cpp/string/basic_string/to_string. I don't see this header explicitly loaded in https://git.rwth-aachen.de/avt-svt/public/libale/-/blob/49bacdeaa0ae4c2236d01a65c8b31f87a8576926/src/tensor.hpp. This looks an upstream bug to me.

[MAiNGO-github]

According to pytorch/pytorch#3192 (comment) there may be a missing

#include <string>

Also C++ docs shows that std::to_string requires the string header file: https://en.cppreference.com/w/cpp/string/basic_string/to_string. I don't see this header explicitly loaded in https://git.rwth-aachen.de/avt-svt/public/libale/-/blob/49bacdeaa0ae4c2236d01a65c8b31f87a8576926/src/tensor.hpp. This looks an upstream bug to me.

I agree that not including <string> is a mistake. In my understanding it is fragile as it implicitly assumes <string> will be included at the place where the template is used.

This has been noted and upstream will fix it in the next release.

However, I believe this is not the reason the compilation failed, but an outdated SDK version.

The first reason is that compilation outside of Binary Builder works with Clang on Linux.
Secondly, further down in the log it complained that std::variant is not yet supported (and here gives explicity that that requires SDK 10.15).
After changing to 10.15, we no longer see the error about std::to_string.

Instead, the problem is, as we had seen previously, #7139, that is shared_pointer to array is not yet supported.

For now I suggest building for MacOS with GCC, which we have seen before works.
Is that fine?

[MAiNGO-github]

@giordano Would you still have access to the logs of the failed pipeline of the lastest commit (8c2ce78) that I reference above, even after I add another commit ?
I would like to put the pull request to the suggested state (=reverting to the situation of d56546f) with passing tests that might be accepted.

[giordano]

Patching the offending file with

diff --git a/M/MAiNGO/build_tarballs.jl b/M/MAiNGO/build_tarballs.jl
index 3a28c6fd9..3e0e32ba0 100644
--- a/M/MAiNGO/build_tarballs.jl
+++ b/M/MAiNGO/build_tarballs.jl
@@ -7,58 +7,52 @@ version = v"0.7.2"
 
 # Collection of sources required to complete build
 sources = [
-    GitSource("https://git.rwth-aachen.de/avt-svt/public/maingo.git", "252733413a29dbe5b84a4cdaf53e60e9934f372f"),
+    GitSource("https://git.rwth-aachen.de/avt-svt/public/maingo.git",
+              "252733413a29dbe5b84a4cdaf53e60e9934f372f"),
+    ArchiveSource("https://github.com/phracker/MacOSX-SDKs/releases/download/10.15/MacOSX10.15.sdk.tar.xz",
+                  "2408d07df7f324d3beea818585a6d990ba99587c218a3969f924dfcc4de93b62"),
+    DirectorySource("./bundled"),
 ]
 
 # Bash recipe for building across all platforms
 script = raw"""
 cd $WORKSPACE/srcdir/maingo/
 git remote set-url origin https://git.rwth-aachen.de/avt-svt/public/maingo.git
-mkdir build
-cd build
-git submodule init
-git submodule update -j 1
+git submodule update --init
 
+atomic_patch -p1 -d dep/libale ../patches/libale-tensor-string.patch
 
-common_cmake_options="-DCMAKE_BUILD_TYPE=Release \
-                     -DMAiNGO_build_standalone=True \
-                     -DMAiNGO_build_shared_c_api=True \
-                     -DMAiNGO_build_parser=True \
-                     -DMAiNGO_use_cplex=False \
-                     -DMAiNGO_use_melon=False"
+common_cmake_options=(
+    -DCMAKE_BUILD_TYPE=Release
+    -DMAiNGO_build_standalone=True
+    -DMAiNGO_build_shared_c_api=True
+    -DMAiNGO_build_parser=True
+    -DMAiNGO_use_cplex=False
+    -DMAiNGO_use_melon=False
+)
 
-# GCC used because of https://github.com/JuliaPackaging/Yggdrasil/issues/7139
 if [[ "${target}" == x86_64-apple-darwin* ]]; then
+    # Install a newer SDK which supports `std::to_string`
+    pushd $WORKSPACE/srcdir/MacOSX10.*.sdk
+    rm -rf /opt/${target}/${target}/sys-root/System
+    cp -ra usr/* "/opt/${target}/${target}/sys-root/usr/."
+    cp -ra System "/opt/${target}/${target}/sys-root/."
+    popd
     export MACOSX_DEPLOYMENT_TARGET=10.15
-#    cmake -DCMAKE_INSTALL_PREFIX=${prefix} \
-#          -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN%.*}_gcc.cmake \
-#          ${common_cmake_options} \
-#          ..
 fi
-#else
-    cmake -DCMAKE_INSTALL_PREFIX=${prefix} \
-          -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN} \
-          ${common_cmake_options} \
-          ..
-#fi
 
+cmake -DCMAKE_INSTALL_PREFIX=${prefix} \
+      -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN} \
+      "${common_cmake_options[@]}" \
+      -B build
 
-
-cmake --build . --config Release --parallel ${nproc}
+cmake --build build --config Release --parallel ${nproc}
 install -Dvm 755 "MAiNGO${exeext}" "${bindir}/MAiNGO${exeext}"
 install -Dvm 755 "MAiNGOcpp${exeext}" "${bindir}/MAiNGOcpp${exeext}"
 install -Dvm 755 "libmaingo-c-api.${dlext}" "${libdir}/libmaingo-c-api.${dlext}"
 install_license ../LICENSE
 """
 
-# These are the platforms we will build for by default, unless further
-# platforms are passed in on the command line
-
-
-#Auditor complains about avx1.
-#Without march the Auditor detects avx2
-#but with march="avx2" avx512 is detected, so we build without march
-
 platforms = supported_platforms()
 #FreeBSD is not supported
 filter!(!Sys.isfreebsd, platforms)
diff --git a/M/MAiNGO/bundled/patches/libale-tensor-string.patch b/M/MAiNGO/bundled/patches/libale-tensor-string.patch
new file mode 100644
index 000000000..720792bee
--- /dev/null
+++ b/M/MAiNGO/bundled/patches/libale-tensor-string.patch
@@ -0,0 +1,12 @@
+diff --git a/src/tensor.hpp b/src/tensor.hpp
+index 2cc67d3..baa8577 100644
+--- a/src/tensor.hpp
++++ b/src/tensor.hpp
+@@ -15,6 +15,7 @@
+ #include <array>
+ #include <stdexcept>
+ #include <vector>
++#include <string>
+ 
+ #include <memory>
+ 

got me past the std::to_string error, but then I get a plethora of other errors, like

[  5%] Building CXX object dep/libale/CMakeFiles/ale.dir/src/parser.cpp.o
cd /workspace/srcdir/maingo/build/dep/libale && /opt/bin/x86_64-apple-darwin14-libgfortran5-cxx11/x86_64-apple-darwin14-clang++ --sysroot=/opt/x86_64-apple-darwin14/x86_64-apple-darwin14/sys-root  -I/workspace/srcdir/maingo/dep/libale/src -O3 -DNDEBUG -mmacosx-version-min=10.15 -fPIC -std=gnu++17 -MD -MT dep/libale/CMakeFiles/ale.dir/src/parser.cpp.o -MF CMakeFiles/ale.dir/src/parser.cpp.o.d -o CMakeFiles/ale.dir/src/parser.cpp.o -c /workspace/srcdir/maingo/dep/libale/src/parser.cpp
In file included from /workspace/srcdir/maingo/dep/libale/src/parser.cpp:12:
In file included from /workspace/srcdir/maingo/dep/libale/src/parser.hpp:26:
In file included from /workspace/srcdir/maingo/dep/libale/src/node.hpp:16:
In file included from /workspace/srcdir/maingo/dep/libale/src/value.hpp:16:
/workspace/srcdir/maingo/dep/libale/src/tensor.hpp:107:16: error: no matching member function for call to 'reset'
        m_data.reset(new TType[size]);
        ~~~~~~~^~~~~
/workspace/srcdir/maingo/dep/libale/src/tensor.hpp:92:9: note: in instantiation of member function 'ale::tensor<double, 3>::tensor' requested here
        tensor(other.cref()) { }
        ^
/workspace/srcdir/maingo/dep/libale/src/symbol.hpp:51:8: note: in instantiation of member function 'ale::tensor<double, 3>::tensor' requested here
struct parameter_symbol;
       ^
/workspace/srcdir/maingo/dep/libale/src/symbol.hpp:159:5: note: in instantiation of member function 'ale::derived_value_symbol<ale::parameter_symbol, ale::tensor_type<ale::base_real, 3>>::clone' requested here
    parameter_symbol(std::string name,
    ^
/workspace/srcdir/maingo/dep/libale/src/parser.tpp:2937:17: note: in instantiation of member function 'ale::parameter_symbol<ale::tensor_type<ale::base_real, 3>>::parameter_symbol' requested here
      name, new parameter_symbol<IteratorType>(name, IsPlaceholderFlag::TRUE));
                ^
/workspace/srcdir/maingo/dep/libale/src/parser.tpp:2804:10: note: in instantiation of function template specialization 'ale::parser::match_internal_function_impl<ale::sum_node<ale::tensor_type<ale::base_real, 3>>, ale::tensor_type<ale::base_real, 0>, ale::tensor_type<ale::base_real, 3>, ale::tensor_type<ale::base_real, 0>>' requested here
  return match_internal_function_impl<NodeType>(
         ^
/workspace/srcdir/maingo/dep/libale/src/parser.tpp:966:7: note: in instantiation of function template specialization 'ale::parser::match_internal_function<ale::sum_node<ale::tensor_type<ale::base_real, 3>>, ale::tensor_type<ale::base_real, 0>>' requested here
      match_internal_function<sum_node<real<IDim>>>(result, "sum") ||
      ^
/workspace/srcdir/maingo/dep/libale/src/parser.tpp:96:14: note: in instantiation of function template specialization 'ale::parser::match_any_sum<3U>' requested here
             match_any_sum<LIBALE_MAX_DIM>(result) ||
             ^
/workspace/srcdir/maingo/dep/libale/src/parser.cpp:248:9: note: in instantiation of function template specialization 'ale::parser::match_primary<ale::tensor_type<ale::base_real, 0>>' requested here
    if(!match_primary(child)) {
        ^
/opt/x86_64-apple-darwin14/x86_64-apple-darwin14/sys-root/usr/include/c++/v1/memory:3910:9: note: candidate template ignored: requirement 'is_convertible<double *, double (*)[]>::value' was not satisfied [with _Yp = double]
        reset(_Yp* __p);
        ^
/opt/x86_64-apple-darwin14/x86_64-apple-darwin14/sys-root/usr/include/c++/v1/memory:3902:10: note: candidate function not viable: requires 0 arguments, but 1 was provided
    void reset() _NOEXCEPT;
         ^
/opt/x86_64-apple-darwin14/x86_64-apple-darwin14/sys-root/usr/include/c++/v1/memory:3918:9: note: candidate function template not viable: requires 2 arguments, but 1 was provided
        reset(_Yp* __p, _Dp __d);
        ^
/opt/x86_64-apple-darwin14/x86_64-apple-darwin14/sys-root/usr/include/c++/v1/memory:3926:9: note: candidate function template not viable: requires 3 arguments, but 1 was provided
        reset(_Yp* __p, _Dp __d, _Alloc __a);
        ^

Ok, let's go back to using GCC, but the fact compilation is successful with a more forgiving compiler isn't an argument for good behaviour of the codebase.

[MAiNGO-github]

The point is not that the compiler is more forgiving. Indeed, Clang can compile the codebase. But only when C++17 is fully supported on the platform. However, the compiler + XCode combination used for MacOS in BinaryBuilder does not implement the whole set of C++17 standard library features. See "std::shared_ptr and std::weak_ptr with array support" under here (https://en.cppreference.com/w/cpp/compiler_support/17).
The error you found with m_data.reset is showing the problem: this function should exist, but the compiler is not fully standard compliant.