Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: SciPy 1.11.0rc1 not buildable on PPC due to boost submodule #18600

Closed
h-vetinari opened this issue Jun 1, 2023 · 9 comments · Fixed by #18630
Closed

BUG: SciPy 1.11.0rc1 not buildable on PPC due to boost submodule #18600

h-vetinari opened this issue Jun 1, 2023 · 9 comments · Fixed by #18630
Labels
Build issues Issues with building from source, including different choices of architecture, compilers and OS C/C++ Items related to the internal C/C++ code base
Milestone

Comments

@h-vetinari
Copy link
Member

Trying to build rc1 for conda-forge in conda-forge/scipy-feedstock#235, things are looking pretty OK with two exceptions. I'll open another issue for the OSX test failures, but on ppc, the build already fails during compilation:

../scipy/_lib/boost_math/include/boost/math/tools/promotion.hpp:140:27: error: static assertion failed: Sorry, but this platform does not have sufficient long double support for the special functions to be reliably implemented.
  140 |          static_assert((0 == std::is_same<type, long double>::value), "Sorry, but this platform does not have sufficient long double support for the special functions to be reliably implemented.");
      |                        ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The boostorg/math submodule (or some recent update thereof) seems to contain a static_assert that unconditionally fails compilation on PPC (or maybe we can toggle off the long double support on PPC?)

PPC is our "most niche" platform, but it's still a hard compilation error on a long-standing arch in conda-forge, so I'm tentatively milestoning this for 1.11.

CC @mckib2 @mdhaber @rgommers @tylerjereddy

@h-vetinari h-vetinari added Build issues Issues with building from source, including different choices of architecture, compilers and OS C/C++ Items related to the internal C/C++ code base labels Jun 1, 2023
@h-vetinari h-vetinari added this to the 1.11.0 milestone Jun 1, 2023
@mborland
Copy link
Contributor

mborland commented Jun 1, 2023

Do you know which long double format your version of PPC64LE is running (e.g. ibm128 or float128)?

@h-vetinari
Copy link
Member Author

I don't actually... It's actually running through QEMU 7.2 on x64 agent. How would I be able to find that out?

@mborland
Copy link
Contributor

mborland commented Jun 1, 2023

Is there a way to output LDBL_MANT_DIG in your config information? Here's the info on the transition: https://developers.redhat.com/articles/2023/05/16/benefits-fedora-38-long-double-transition-ppc64le#. I'll look through the runner and see if I can find anything.

@h-vetinari
Copy link
Member Author

Here's the info on the transition: https://developers.redhat.com/articles/2023/05/16/benefits-fedora-38-long-double-transition-ppc64le#

Then it's almost certainly the IBM one. We're not on anything nearly as new as Fedora 36 (rather, something like CentOS 7).

PS. Thanks a lot for taking a look at this so quickly, much appreciated!

@rgommers
Copy link
Member

rgommers commented Jun 1, 2023

Here's a larger traceback so we see what code tries to pull this in:

2023-06-01T03:46:39.8765835Z [837/1628] Compiling C++ object scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/meson-generated_binom_ufunc.cpp.o
2023-06-01T03:46:39.8774507Z FAILED: scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/meson-generated_binom_ufunc.cpp.o 
2023-06-01T03:46:39.8796606Z $BUILD_PREFIX/bin/powerpc64le-conda-linux-gnu-c++ -Iscipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p -Iscipy/stats/_boost -I../scipy/stats/_boost -I../scipy/stats/_boost/include -I../scipy/_lib/boost_math/include -I../../../_build_env/venv/lib/python3.9/site-packages/numpy/core/include -I$PREFIX/include/pypy3.9 -fvisibility=hidden -fvisibility-inlines-hidden -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++14 -O3 -fvisibility-inlines-hidden -fmessage-length=0 -mcpu=power8 -mtune=power8 -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O3 -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/scipy-split-1.11.0rc1 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -fvisibility-inlines-hidden -fmessage-length=0 -mcpu=power8 -mtune=power8 -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O3 -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/scipy-split-1.11.0rc1 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -O2 -isystem $PREFIX/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem $PREFIX/include -fPIC -DBOOST_MATH_STANDALONE=1 -DBOOST_MATH_DOMAIN_ERROR_POLICY=ignore_error -DBOOST_MATH_EVALUATION_ERROR_POLICY=user_error -DBOOST_MATH_OVERFLOW_ERROR_POLICY=user_error -DBOOST_MATH_PROMOTE_DOUBLE_POLICY=false -DNPY_NO_DEPRECATED_API=NPY_1_9_API_VERSION -Wno-cpp -MD -MQ scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/meson-generated_binom_ufunc.cpp.o -MF scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/meson-generated_binom_ufunc.cpp.o.d -o scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/meson-generated_binom_ufunc.cpp.o -c scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/binom_ufunc.cpp
2023-06-01T03:46:39.8800506Z In file included from ../scipy/_lib/boost_math/include/boost/math/special_functions/detail/round_fwd.hpp:12,
2023-06-01T03:46:39.8856079Z                  from ../scipy/_lib/boost_math/include/boost/math/special_functions/math_fwd.hpp:29,
2023-06-01T03:46:39.8856743Z                  from ../scipy/_lib/boost_math/include/boost/math/special_functions/fpclassify.hpp:18,
2023-06-01T03:46:39.8857230Z                  from ../scipy/_lib/boost_math/include/boost/math/distributions/detail/common_error_handling.hpp:13,
2023-06-01T03:46:39.8857651Z                  from ../scipy/_lib/boost_math/include/boost/math/distributions/arcsine.hpp:35,
2023-06-01T03:46:39.8858075Z                  from ../scipy/_lib/boost_math/include/boost/math/distributions.hpp:15,
2023-06-01T03:46:39.8858466Z                  from $SRC_DIR/base/scipy/stats/_boost/include/func_defs.hpp:7,
2023-06-01T03:46:39.8859050Z                  from scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/binom_ufunc.cpp:777:
2023-06-01T03:46:39.8859819Z ../scipy/_lib/boost_math/include/boost/math/tools/promotion.hpp: In instantiation of 'struct boost::math::tools::promote_args<long double, long double, long double, float, float, float>':
2023-06-01T03:46:39.8861406Z ../scipy/_lib/boost_math/include/boost/math/special_functions/beta.hpp:1603:4:   required by substitution of 'template<class RT1, class RT2, class RT3, class Policy> typename boost::math::tools::promote_args<RT1, RT2, A>::type boost::math::ibeta_derivative(RT1, RT2, RT3, const Policy&) [with RT1 = long double; RT2 = long double; RT3 = long double; Policy = boost::math::policies::policy<boost::math::policies::discrete_quantile<boost::math::policies::integer_round_up> >]'
2023-06-01T03:46:39.8862901Z ../scipy/_lib/boost_math/include/boost/math/distributions/binomial.hpp:498:32:   required from 'RealType boost::math::pdf(const binomial_distribution<RealType, Policy>&, const RealType&) [with RealType = long double; Policy = policies::policy<policies::discrete_quantile<boost::math::policies::integer_round_up> >]'
2023-06-01T03:46:39.8863976Z $SRC_DIR/base/scipy/stats/_boost/include/func_defs.hpp:64:32:   required from 'RealType boost_pdf(RealType, const Args ...) [with Dst = boost::math::binomial_distribution; RealType = long double; Args = {long double, long double}]'
2023-06-01T03:46:39.8864747Z scipy/stats/_boost/binom_ufunc.pypy39-pp73-ppc_64-linux-gnu.so.p/binom_ufunc.cpp:3329:61:   required from here
2023-06-01T03:46:39.8865338Z ../scipy/_lib/boost_math/include/boost/math/tools/promotion.hpp:140:27: error: static assertion failed: Sorry, but this platform does not have sufficient long double support for the special functions to be reliably implemented.
2023-06-01T03:46:39.8866226Z   140 |          static_assert((0 == std::is_same<type, long double>::value), "Sorry, but this platform does not have sufficient long double support for the special functions to be reliably implemented.");

I'm traveling right now so hard to check, but it'd be great to know if this is a regression in Boost.Math, or if it came in when we replaced a scipy.special or scipy.stats function with a Boost implementation, or if we added long double support as a new feature for 1.11.0 (perhaps by accident).

@jzmaddock
Copy link

If you look at the backtrace you'll see that you are explicitly calling the pdf on binomial_distribution<long double>, so if it's a regression I would say it's at your end. Probably ;)

BTW the reason we treat this as an error is that "double double"'s cause all kinds of issues in some of our code - it's simply not possible to reason about their behaviour in a sensible way because they have a "discontinuous" representation, this is reflected in their value for numeric_limits<long double>::epsilon() which is typically numeric_limits<double>::min(). This is technically correct in that it's the smallest value you can add to 1 and effect a change, but functionally useless in any other sense.

So... my contention has always been that it's just too much work to make these types function correctly inside Boost.Math, but I'm happy to be proved wrong!

@rgommers
Copy link
Member

rgommers commented Jun 1, 2023

So... my contention has always been that it's just too much work to make these types function correctly inside Boost.Math, but I'm happy to be proved wrong!

I completely agree, and I'd like to remove long double support from SciPy (and NumPy) completely. It's not very useful, and full of problems implementation/maintenance-wise. We just can't rip it out with zero notice unfortunately, so we should try to solve the problem here. gh-16185 already took a pragmatic approach in not supporting long double on Windows (it'd be 64-bit anyway there).

@mckib2
Copy link
Contributor

mckib2 commented Jun 4, 2023

float_types.append('NPY_LONGDOUBLE')

Looks like in stats we generate a NPY_LONGDOUBLE override on all platform except Windows. Seems like there's an easy fix to remove the long double code generation -- can we do this without a deprecation cycle? Seems like we're accidentally supporting this at this point

@h-vetinari
Copy link
Member Author

Thanks for everyone's input! I tested @mckib2's #18630 in conda-forge/scipy-feedstock#235 and compilation on PPC now passes. 🥳

If we need to keep long double for a while longer (if only to deprecate it), then we could easily restrict that patch to PPC (either here or downstream in conda-forge).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build issues Issues with building from source, including different choices of architecture, compilers and OS C/C++ Items related to the internal C/C++ code base
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants