scipy: BUG: 1.12.0rc1: build failure on windows due to macro collision in boost

It looks like boost is using an unfortunate choice of template variable name on windows, which conflicts with a macro:

In file included from ..\scipy\_lib\boost_math\include\boost/math/policies/policy.hpp:11:
..\scipy\_lib\boost_math\include\boost/math/tools/mp.hpp(171,34): error: expected parameter declarator
  171 | template<typename L, std::size_t I>
      |                                  ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\complex.h(63,20): note: expanded from macro 'I'
   63 | #define I          _Complex_I
      |                    ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\complex.h(62,29): note: expanded from macro '_Complex_I'
   62 | #define _Complex_I _FCbuild(0.0F, 1.0F)
      |                             ^

From https://github.com/conda-forge/scipy-feedstock/pull/262, using the vendored boost.

CC @rgommers @tylerjereddy

About this issue

Original URL
State: closed
Created 6 months ago
Comments: 32 (31 by maintainers)

Most upvoted comments

Haven’t checked previous versions, but C++20 says: “The C headers <complex.h> and <tgmath.h> do not contain any of the content from the C standard library and instead merely include other headers from the C++ standard library.”

So, even if there is a stray include of complex.h somewhere, it should not be defining the macro I.

That said, we should probably protect ourselves from such things, given that we also take care over min/max and other things which shouldn’t be macros.

I have a workaround here: https://github.com/boostorg/math/pull/1060 that’s not unlike our protection for MINMAX.

The working draft of the C++ ISO updated the wording to: The header <complex.h> behaves as if it simply includes the header <complex> so this looks like an MSVC STL bug.

mborland on Dec 21, 2023

I went from https://docs.scipy.org/doc/scipy/dev/toolchain.html#c-language-standards:

as of mid-2022, support for the entirety of the C++17 standard library has not yet been completed across all compilers

to its reference, https://en.cppreference.com/w/cpp/compiler_support/17#C.2B.2B17_library_features, which seems to have a few yellow and red boxes still.

The distinction between language and standard library is important. We can use C++17 as a language version (fully supported by every compiler we use), as long as we don’t use the small handful of not-universally-implemented stdlib features.

h-vetinari on Jan 6, 2024

So with the following changes, SciPy 1.12.0rc1 builds fine and passes the tests in conda-forge:

Use -Dcpp_std=c++17
Backport https://github.com/scipy/scipy/pull/19761
Keep skip for https://github.com/scipy/scipy/issues/15533
Bump tolerances in two failing tests (details below)

Tolerances patch

commit 2ab23928c71697410d45f2c9e97e77b141943737
Author: @h-vetinari <h.vetinari@gmx.com>
Date:   Fri Dec 29 13:34:40 2023 +1100

    loosen tolerances for tests that fail otherwise on windows+MKL

diff --git a/scipy/optimize/tests/test__differential_evolution.py b/scipy/optimize/tests/test__differential_evolution.py
index 6ea6100b4..c3ad7dc2a 100644
--- a/scipy/optimize/tests/test__differential_evolution.py
+++ b/scipy/optimize/tests/test__differential_evolution.py
@@ -1013,9 +1013,9 @@ class TestDifferentialEvolutionSolver:
         x_opt = (1, 1, 1, 1, 1, 1, 1, 1, 1, 3, 3, 3, 1)
         f_opt = -15

-        assert_allclose(f(x_opt), f_opt)
+        assert_allclose(f(x_opt), f_opt, atol=6e-4)
         assert res.success
-        assert_allclose(res.x, x_opt, atol=5e-4)
+        assert_allclose(res.x, x_opt, atol=6e-4)
         assert_allclose(res.fun, f_opt, atol=5e-3)
         assert_(np.all(A@res.x <= b))
         assert_(np.all(res.x >= np.array(bounds)[:, 0]))
@@ -1063,7 +1063,7 @@ class TestDifferentialEvolutionSolver:
                                          seed=1234, constraints=constraints,
                                          popsize=2)

-        assert_allclose(res.x, x_opt, atol=5e-4)
+        assert_allclose(res.x, x_opt, atol=6e-4)
         assert_allclose(res.fun, f_opt, atol=5e-3)
         assert_(np.all(A@res.x <= b))
         assert_(np.all(res.x >= np.array(bounds)[:, 0]))
diff --git a/scipy/stats/tests/test_distributions.py b/scipy/stats/tests/test_distributions.py
index 6268cef88..3aa62e75d 100644
--- a/scipy/stats/tests/test_distributions.py
+++ b/scipy/stats/tests/test_distributions.py
@@ -4130,9 +4130,9 @@ class TestGenExpon:
                               (0.125, 0.9508674287164001, 0.25, 5, 0.5)])
     def test_sf_isf(self, x, p, a, b, c):
         sf = stats.genexpon.sf(x, a, b, c)
-        assert_allclose(sf, p, rtol=1e-14)
+        assert_allclose(sf, p, rtol=2e-14)
         isf = stats.genexpon.isf(p, a, b, c)
-        assert_allclose(isf, x, rtol=1e-14)
+        assert_allclose(isf, x, rtol=2e-14)

     # The values of p in the following data were computed with mpmath.
     @pytest.mark.parametrize('x, p, a, b, c',
@@ -4143,9 +4143,9 @@ class TestGenExpon:
                               (0.125, 0.04913257128359998, 0.25, 5, 0.5)])
     def test_cdf_ppf(self, x, p, a, b, c):
         cdf = stats.genexpon.cdf(x, a, b, c)
-        assert_allclose(cdf, p, rtol=1e-14)
+        assert_allclose(cdf, p, rtol=2e-14)
         ppf = stats.genexpon.ppf(p, a, b, c)
-        assert_allclose(ppf, x, rtol=1e-14)
+        assert_allclose(ppf, x, rtol=2e-14)


 class TestTruncexpon:

h-vetinari on Jan 2, 2024

So, unless I’ve made an obvious (or at least fixable) mistake, I think the only remaining option is to unconditionally turn off aligned_alloc support in pocketfft on windows.

Your logic looks correct to me so I think removing support on windows is the best bet.

mborland on Dec 27, 2023

Yeah, I know, but we don’t do this consistently, and meson generally takes care of this for us. It’s only that it doesn’t do so for clang-cl: mesonbuild/meson#12670

See: https://github.com/scipy/scipy/pull/19761

mborland on Dec 26, 2023