#32021 closed defect (fixed)

numpy 1.20* inconsistent in recognising CPU features

Reported by: Dima Pasechnik Owned by:
Priority: major Milestone: sage-9.4
Component: build Keywords:
Cc: gh-kliem, David Coudert, Frédéric Chapoton Merged in:
Authors: Jonathan Kliem Reviewers: John Palmieri, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: a7eef1b (Commits, GitHub, GitLab) Commit: a7eef1b2357cd3862facf64edb81abdafbda6923
Dependencies: Stopgaps:

Status badges

Description

typically this looks like the following: https://groups.google.com/d/msgid/sage-devel/763f8650-9803-4bba-a0ec-46744204fc22n%40googlegroups.com?utm_medium=email&utm_source=footer

[sagelib-9.4.beta2]   File "/home/chapoton/sage/local/lib/python3.8/site-packages/numpy/core/overrides.py", line 7, in <module>
[sagelib-9.4.beta2]     from numpy.core._multiarray_umath import (
[sagelib-9.4.beta2] RuntimeError: NumPy was built with baseline optimizations: 
[sagelib-9.4.beta2] (SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42) but your machine doesn't support:
[sagelib-9.4.beta2] (POPCNT).

Change History (57)

comment:1 Changed 18 months ago by Dima Pasechnik

Cc: gh-kliem added
Component: PLEASE CHANGEbuild
Type: PLEASE CHANGEdefect

comment:2 Changed 18 months ago by Dima Pasechnik

See also https://groups.google.com/g/sage-release/c/lmUFq4d7is0/m/uKRsE9KoBQAJ

It could be that builds in question actually took a prebuilt on another machine numpy, then it's a numpy bug, I guess, to install an incompatible binary.

comment:3 Changed 18 months ago by Dima Pasechnik

Cc: David Coudert Frédéric Chapoton added

comment:4 Changed 18 months ago by Matthias Köppe

Status: newneeds_info

comment:5 Changed 18 months ago by John Palmieri

I see this when building from scratch on an iMac Pro, OS X 10.15.7, with various homebrew packages installed. numpy is built from scratch and says

########### EXT COMPILER OPTIMIZATION ###########
Platform      : 
  Architecture: x64
  Compiler    : gcc

CPU baseline  : 
  Requested   : 'native'
  Enabled     : SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42 AVX F16C FMA3 AVX2 AVX512F AVX512CD AVX512_SKX
  Flags       : -msse -msse2 -msse3 -mssse3 -msse4.1 -mpopcnt -msse4.2 -mavx -mf16c -mfma -mavx2 -mavx512f -mavx512cd -mavx512vl -mavx512bw -mavx512dq
  Extra checks: AVX512F_REDUCE AVX512BW_MASK AVX512DQ_MASK

CPU dispatch  : 
  Requested   : 'max -xop -fma4'
  Enabled     : AVX512_KNL AVX512_CLX AVX512_CNL AVX512_ICL
  Generated   : none
CCompilerOpt._cache_write[796] : write cache to path -> /Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta2/local/var/tmp/sage/build/numpy-1.20.3/src/build/temp.macosx-10.15-x86_64-3.9/ccompiler_opt_cache_ext.py

########### CLIB COMPILER OPTIMIZATION ###########
Platform      : 
  Architecture: x64
  Compiler    : gcc

CPU baseline  : 
  Requested   : 'native'
  Enabled     : SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42 AVX F16C FMA3 AVX2 AVX512F AVX512CD AVX512_SKX
  Flags       : -msse -msse2 -msse3 -mssse3 -msse4.1 -mpopcnt -msse4.2 -mavx -mf16c -mfma -mavx2 -mavx512f -mavx512cd -mavx512vl -mavx512bw -mavx512dq
  Extra checks: AVX512F_REDUCE AVX512BW_MASK AVX512DQ_MASK

CPU dispatch  : 
  Requested   : 'max -xop -fma4'
  Enabled     : AVX512_KNL AVX512_CLX AVX512_CNL AVX512_ICL
  Generated   : none

scipy fails with

    RuntimeError: NumPy was built with baseline optimizations:
    (SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42 AVX F16C FMA3 AVX2 AVX512F AVX512CD AVX512_SKX) but your machine doesn't support:
    (AVX512F).

I notice that we deleted related numpy patches in the latest update, presumably because they were included into the source code. There must have been other changes in the source that broke something else.

comment:6 Changed 18 months ago by John Palmieri

I cannot build Sage on this machine right now; does that this ticket a blocker, or is it too architecture-specific? (I can build on some other OS X machines just fine.)

comment:7 Changed 18 months ago by gh-kliem

If you add --cpu-baseline="sse2" to `spkg-install. Will this work? Similar to #31565.

comment:8 in reply to:  6 ; Changed 18 months ago by Dima Pasechnik

Replying to jhpalmieri:

I cannot build Sage on this machine right now; does that this ticket a blocker, or is it too architecture-specific? (I can build on some other OS X machines just fine.)

Can you do a standalone build of scipy+numpy on that machine? Or does it fail in the same way?

comment:9 in reply to:  8 Changed 18 months ago by John Palmieri

Replying to dimpase:

Replying to jhpalmieri:

I cannot build Sage on this machine right now; does that this ticket a blocker, or is it too architecture-specific? (I can build on some other OS X machines just fine.)

Can you do a standalone build of scipy+numpy on that machine? Or does it fail in the same way?

I can't easily mimic the Sage install. If I just unpack the zip file for numpy and run python3 setup.py build -j 4 install --prefix $HOME/.local, it complains about blas stuff:

RuntimeError: Found /usr/lib/libcblas.dylib, but that file is a symbolic link to the MacOS Accelerate framework, which is not supported by NumPy. You must configure the build to use a different optimized library, or disable the use of optimized BLAS and LAPACK by setting the environment variables NPY_BLAS_ORDER="" and NPY_LAPACK_ORDER="" before building NumPy.

I tried running Sage's lapack_conf.py, but it complained about not finding cblas. numpy installed after export NPY_BLAS_ORDER="" and export NPY_LAPACK_ORDER="", but scipy did not.

So I don't understand how to build numpy+scipy on this machine.

comment:10 in reply to:  7 ; Changed 18 months ago by John Palmieri

Replying to gh-kliem:

If you add --cpu-baseline="sse2" to `spkg-install. Will this work? Similar to #31565.

I tried

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..5d45f176ae 100644
    a b python3 ../lapack_conf.py 
    2222export FFLAGS="$FFLAGS -fPIC"
    2323export FCFLAGS="$FCFLAGS -fPIC"
    2424
    25 export NUMPY_FCONFIG="config_fc --noopt --noarch"
     25export NUMPY_FCONFIG="config_fc --noopt --noarch --cpu-baseline=\"sse2\""
    2626
    2727################################################
    2828

but this isn't the right syntax:

usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

error: option --cpu-baseline not recognized

Can you provide more guidance?

comment:11 in reply to:  10 ; Changed 18 months ago by gh-kliem

Replying to jhpalmieri:

Replying to gh-kliem:

If you add --cpu-baseline="sse2" to `spkg-install. Will this work? Similar to #31565.

I tried

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..5d45f176ae 100644
    a b python3 ../lapack_conf.py 
    2222export FFLAGS="$FFLAGS -fPIC"
    2323export FCFLAGS="$FCFLAGS -fPIC"
    2424
    25 export NUMPY_FCONFIG="config_fc --noopt --noarch"
     25export NUMPY_FCONFIG="config_fc --noopt --noarch --cpu-baseline=\"sse2\""
    2626
    2727################################################
    2828

but this isn't the right syntax:

usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

error: option --cpu-baseline not recognized

Can you provide more guidance?

Thanks for catching this. Actually you need to do it during a different command:

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..c261d89444 100644
    a b export NUMPY_FCONFIG="config_fc --noopt --noarch" 
    2828
    2929sdh_setup_bdist_wheel ${NUMPY_FCONFIG}
    3030
    31 sdh_store_and_pip_install_wheel .
     31sdh_store_and_pip_install_wheel . --cpu-baseline="sse2"

A cleaner solution and also a good solution for a branch might be to disable -march=native via

export CFLAGS="$CFLAGS_NON_NATIVE"

Numpy will still compile intrinsics to be dispatched on runtime. (And even if it isn't the case, the policy was that -march=native should only be enabled as long as this does not effect portability). Either way it appears to be a numpy bug, because it detects your architecture wrongly.

I hope one of those things work.

Edited: See comment below

Last edited 18 months ago by gh-kliem (previous) (diff)

comment:12 Changed 18 months ago by gh-kliem

Actually you don't want cpu-dispatch but cpu-baseline as before.

comment:13 in reply to:  11 ; Changed 18 months ago by John Palmieri

Replying to gh-kliem:

Thanks for catching this. Actually you need to do it during a different command:

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..c261d89444 100644
    a b export NUMPY_FCONFIG="config_fc --noopt --noarch" 
    2828
    2929sdh_setup_bdist_wheel ${NUMPY_FCONFIG}
    3030
    31 sdh_store_and_pip_install_wheel .
     31sdh_store_and_pip_install_wheel . --cpu-baseline="sse2"

This doesn't work, unfortunately:

Error: sdh_store_wheel requires .  as only argument

Can we build numpy using other of Sage's helpers which accept arguments?

A cleaner solution and also a good solution for a branch might be to disable -march=native via

export CFLAGS="$CFLAGS_NON_NATIVE"

I tried various options including

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..88aee50f6a 100644
    a b if [ `uname` = "Darwin" ]; then 
    66    unset ATLAS
    77    unset BLAS
    88    unset LAPACK
     9    export CFLAGS="$CFLAGS_NON_NATIVE"
     10    export FCFLAGS="$FCFLAGS_NON_NATIVE"
     11    export FFLAGS="$FFLAGS_NON_NATIVE"
    912else
    1013    export {ATLAS,PTATLAS,OPENBLAS,MKL,MKLROOT}=None
    1114    export LDFLAGS="${LDFLAGS} -shared"

but they didn't change the "COMPILER OPTIMIZATION" summaries when building numpy and they did not allow scipy to build.

comment:14 Changed 18 months ago by gh-kliem

You are exactly right. The previous place was the correct place to use it.

export NUMPY_FCONFIG="config_fc --noopt --noarch --cpu-baseline=\"sse2\""

However:

The command arguments are available in build, build_clib, and build_ext. if build_clib or build_ext are not specified by the user, the arguments of build will be used instead, which also holds the default values.

So this does not work for bdist_wheel and this is what we use. Apparently, when build with wheel it just gets optimized regardless?

comment:15 in reply to:  13 Changed 18 months ago by gh-kliem

Replying to jhpalmieri:

Replying to gh-kliem:

Thanks for catching this. Actually you need to do it during a different command:

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..c261d89444 100644
    a b export NUMPY_FCONFIG="config_fc --noopt --noarch" 
    2828
    2929sdh_setup_bdist_wheel ${NUMPY_FCONFIG}
    3030
    31 sdh_store_and_pip_install_wheel .
     31sdh_store_and_pip_install_wheel . --cpu-baseline="sse2"

This doesn't work, unfortunately:

Error: sdh_store_wheel requires .  as only argument

Can we build numpy using other of Sage's helpers which accept arguments?

A cleaner solution and also a good solution for a branch might be to disable -march=native via

export CFLAGS="$CFLAGS_NON_NATIVE"

I tried various options including

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..88aee50f6a 100644
    a b if [ `uname` = "Darwin" ]; then 
    66    unset ATLAS
    77    unset BLAS
    88    unset LAPACK
     9    export CFLAGS="$CFLAGS_NON_NATIVE"
     10    export FCFLAGS="$FCFLAGS_NON_NATIVE"
     11    export FFLAGS="$FFLAGS_NON_NATIVE"
    912else
    1013    export {ATLAS,PTATLAS,OPENBLAS,MKL,MKLROOT}=None
    1114    export LDFLAGS="${LDFLAGS} -shared"

but they didn't change the "COMPILER OPTIMIZATION" summaries when building numpy and they did not allow scipy to build.

As Matthias Köppe pointed out you can do the following change:

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..ef2a322ecb 100644
    a b export NUMPY_FCONFIG="config_fc --noopt --noarch" 
    2626
    2727################################################
    2828
    29 sdh_setup_bdist_wheel ${NUMPY_FCONFIG}
     29sdh_setup_bdist_wheel build ${NUMPY_FCONFIG}
    3030
    3131sdh_store_and_pip_install_wheel .

Once you have done this, you can add all the previous options (added to NUMPY_FCONFIG):

  • cpu-baseline=min,
  • cpu-dispatch
  • try again export CFLAGS=CFLAGS_NON_NATIVE.

Actually you can cancel the numpy built once it starts compiling to immediatly see the optimization options as in comment:5

comment:16 Changed 18 months ago by John Palmieri

Sorry, I'm still not getting it. I made the change suggested in comment:15, adding "build" to the command. If I use

export NUMPY_FCONFIG="config_fc --noopt --noarch --cpu-baseline=\"sse2\""

I still get "error: option --cpu-baseline not recognized". Same with --cpu-dispatch if it's at the end. If I instead use

export NUMPY_FCONFIG="--cpu-baseline=\"sse2\" config_fc --noopt --noarch"

then numpy builds but with no apparent difference to the outcome, and scipy still fails. This happens whether or not I include CFLAGS="$CFLAGS_NON_NATIVE". Note that no matter what I try, the following appears several times in the log file:

CCompilerOpt.__init__[2103] : native flag is specified through environment variables. force cpu-baseline='native'

I don't know if that's relevant.

comment:17 Changed 18 months ago by John Palmieri

This worked for me, until we find something better:

  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..50aa1529b3 100644
    a b python3 ../lapack_conf.py 
    2222export FFLAGS="$FFLAGS -fPIC"
    2323export FCFLAGS="$FCFLAGS -fPIC"
    2424
     25export CFLAGS=" -march=skylake"
     26
    2527export NUMPY_FCONFIG="config_fc --noopt --noarch"
    2628
    2729################################################

comment:19 in reply to:  18 ; Changed 18 months ago by Dima Pasechnik

comment:20 in reply to:  19 Changed 17 months ago by John Palmieri

Replying to dimpase:

Replying to gh-sheerluck:

https://github.com/numpy/numpy/issues/19067

good catch!

John, does the patch from there, https://patch-diff.githubusercontent.com/raw/numpy/numpy/pull/19074.diff fix your issue?

Unfortunately not. I see the same summary as in comment:5, and scipy fails to build.

comment:21 Changed 17 months ago by John Palmieri

By the way, where is CFLAGS_NON_NATIVE set? If I put echo $CFLAGS and echo $CFLAGS_NON_NATIVE in spkg-install.in, they both print the same thing:

-O2 -g -march=native

I'm not sure what "NON_NATIVE" means, but is it odd that it includes -march=native? If I explicitly do export CFLAGS="-O2 -g", then scipy builds.

comment:22 Changed 17 months ago by gh-kliem

Do you have default CFLAGS? User settings take precedence over those. CFLAGS_NON_NATIVE means we don't add -march=native. That's what I figured is going on by the way.

If you don't have CFLAGS set by default than this is a bug.

comment:23 Changed 17 months ago by gh-kliem

The configured flags are in sage/build/bin/sage-build-env-config.

In sage/build/bin/sage/sage-build-env we set the flags. Priorities are in this order:

  1. Whatever is currently set.
  2. During configuration.
  3. Sage's default.

CFLAGS_NON_NATIVE only is meaningful for the third one. Otherwise you choice will be set everywhere regardless (some packages might add something).

comment:24 Changed 17 months ago by John Palmieri

I inserted more echo commands. echo $CFLAGS_NON_NATIVE returns -O2 -g when I print in sage-spkg, but it returns -O2 -g -march=native when this echo command is the first thing in numpy's spkg-install.in script. That is, my numpy log file looks like:

...
Package 'numpy' is currently not installed
Uninstalling 'numpy' with legacy uninstaller
!!!!!!!!!!!!!!!!
PRINTED BY sage-spkg
cflags: -O2 -g -march=native
non-native: -O2 -g
!!!!!!!!!!!!!!!!
!!!!!!!!!!!!!!!!
PRINTED BY NUMPY's spkg-install.in
cflags: -O2 -g -march=native
non-native: -O2 -g -march=native
!!!!!!!!!!!!!!!!
...

After the installation is complete, one more echo command in sage-spkg returns -O2 -g again.


The problem seems to be that we (in the full script spkg-install, created from spkg-install.in) also run the script build/bin/sage-build-env, and the line

    export CFLAGS_NON_NATIVE="$ORIGINAL_CFLAGS"

produces the wrong answer.

Last edited 17 months ago by John Palmieri (previous) (diff)

comment:25 Changed 17 months ago by John Palmieri

One question would be, since sage-spkg already has the lines

. sage-build-env-config
. sage-build-env

why does it also need to write this into spkg-install:

for lib in "\$SAGE_ROOT/build/bin/sage-dist-helpers" "\$SAGE_SRC/bin/sage-env-config" "\$SAGE_SRC/bin/sage-env" "\$SAGE_ROOT/build/bin/sage-build-env-config" "\$SAGE_ROOT/build/bin/sage-build-env"; do
    source "\$lib"
    if [ \$? -ne 0 ]; then
        echo >&2 "Error: failed to source \$lib"
        echo >&2 "Is \$SAGE_ROOT the correct SAGE_ROOT?"
        exit 1
    fi
done

Should we omit sage-build-env-config and sage-build-env from this list?

comment:26 Changed 17 months ago by Matthias Köppe

I think the idea is that these wrapped scripts can also be executed manually

comment:27 Changed 17 months ago by John Palmieri

Sage builds on this machine with these changes:

  • build/bin/sage-spkg

    diff --git a/build/bin/sage-spkg b/build/bin/sage-spkg
    index c92f315f36..561e8f5cce 100755
    a b export PKG_NAME="$PKG_NAME" 
    525525export PKG_BASE="$PKG_BASE"
    526526export PKG_VER="$PKG_VER"
    527527
    528 for lib in "\$SAGE_ROOT/build/bin/sage-dist-helpers" "\$SAGE_SRC/bin/sage-env-config" "\$SAGE_SRC/bin/sage-env" "\$SAGE_ROOT/build/bin/sage-build-env-config" "\$SAGE_ROOT/build/bin/sage-build-env"; do
     528for lib in "\$SAGE_ROOT/build/bin/sage-dist-helpers" "\$SAGE_SRC/bin/sage-env-config" "\$SAGE_SRC/bin/sage-env"; do
    529529    source "\$lib"
    530530    if [ \$? -ne 0 ]; then
    531531        echo >&2 "Error: failed to source \$lib"
  • build/pkgs/numpy/spkg-install.in

    diff --git a/build/pkgs/numpy/spkg-install.in b/build/pkgs/numpy/spkg-install.in
    index 1237d059d3..0eb81094db 100644
    a b python3 ../lapack_conf.py 
    2222export FFLAGS="$FFLAGS -fPIC"
    2323export FCFLAGS="$FCFLAGS -fPIC"
    2424
     25export CFLAGS="$CFLAGS_NON_NATIVE"
     26
    2527export NUMPY_FCONFIG="config_fc --noopt --noarch"
    2628
    2729################################################

comment:28 in reply to:  26 Changed 17 months ago by John Palmieri

Replying to mkoeppe:

I think the idea is that these wrapped scripts can also be executed manually

I guess they should be written so that they're idempotent.

comment:29 Changed 17 months ago by gh-kliem

Yes, that is what I was about to suggest.

comment:30 Changed 17 months ago by Matthias Köppe

This may be a good occasion to move all if's from sage-build-env-config to sage-build-env.

comment:31 Changed 17 months ago by John Palmieri

I fear that I may have derailed the ticket from its original purpose. The patch in comment:19 looks quite relevant to that so maybe we should include it, even though it doesn't fix the problem on my machine. Separate ticket for changes to sage-build-env and sage-build-env-config?

comment:32 Changed 17 months ago by gh-kliem

Authors: Jonathan Kliem
Branch: public/32021
Commit: b3e2a534bd6f4d3d164b0bf8ad0ab33b64cd282f
Status: needs_infoneeds_review

Feel free to add yourself as the author. I don't know at this point, who really is the author here.


New commits:

07a66e0header guard for sage-build-env
ba795b5move ifs from sage-build-env-config to sage-build-env
b3e2a53disable march=native for numpy

comment:33 Changed 17 months ago by John Palmieri

By the way, I don't know if numpy is right or if scipy is right about my machine: maybe scipy is wrong when it says that AVX512F is not supported? How to know for sure?

comment:34 Changed 17 months ago by gh-kliem

lscpu or echo | gcc -dM -E - -march=native.

comment:35 Changed 17 months ago by git

Commit: b3e2a534bd6f4d3d164b0bf8ad0ab33b64cd282f9c87b6571ec5bb9a4dce5f2761906b0110e2709a

Branch pushed to git repo; I updated commit sha1. New commits:

9c87b65correct explanation as popcnt wasnt the problem here

comment:36 Changed 17 months ago by John Palmieri

If I run echo | gcc -dM -E - -march=native, it prints many things including

#define __AVX512F__ 1

OS X doesn't include lscpu, but if I run sysctl -a | grep machdep.cpu, it tells me

machdep.cpu.leaf7_features: RDWRFSGS TSC_THREAD_OFFSET BMI1 HLE AVX2 FDPEO SMEP BMI2 ERMS INVPCID RTM PQM FPU_CSDS MPX PQE AVX512F AVX512DQ RDSEED ADX SMAP CLFSOPT CLWB IPT AVX512CD AVX512BW AVX512VL MDCLEAR TSXFA IBRS STIBP L1DF SSBD

Does this mean that AVX512F is a feature, so scipy is wrong?

comment:37 Changed 17 months ago by gh-kliem

That does look like scipy is wrong.

comment:38 Changed 17 months ago by John Palmieri

The error arises when doing import numpy in Python, which scipy must try to do early in its build process. So it's not really scipy, it's within numpy. I don't know how to track this down.

comment:39 Changed 17 months ago by gh-kliem

It happens here:

https://github.com/numpy/numpy/blob/623bc1fae1d47df24e7f1e29321d0c0ba2771ce0/numpy/core/src/common/npy_cpu_features.c.src

Around line 160. For some reason this check does not work for AVX512F in your case. So yes, this is a numpy bug.

comment:41 Changed 17 months ago by David Coudert

With this patch, I'm able to compile on a Intel MacBook? Air with macOS 10.15.7. I will run doctests asap.

comment:42 Changed 17 months ago by John Palmieri

Works for me, too, and doctests pass as well.

comment:43 Changed 17 months ago by John Palmieri

I would like someone with more familiarity with the build scripts to look over those changes. Also, there is now a NumPy bugfix for my particular problem: https://github.com/numpy/numpy/pull/19362. I think that it is safest to use export CFLAGS="$CFLAGS_NON_NATIVE" for now when building NumPy and not worry about using this patch, but at some point we should investigate whether they've fixed all of the relevant problems.

comment:44 Changed 17 months ago by Matthias Köppe

This PR has been backported to 1.21.x as https://github.com/numpy/numpy/pull/19365, but there is no indication whether it will also be backported to 1.20.x. Have you tried whether the patch applies?

comment:45 Changed 17 months ago by Matthias Köppe

src/bin/sage-env-config.in uses SAGE_ENV_CONFIG_SOURCED and src/bin/sage-env uses SAGE_ENV_SOURCED, so probably SAGE_HAS_RUN_SAGE_BUILD_ENV should be renamed to follow the same pattern.

Otherwise the changes to sage-build-env, sage-build-env-config.in look good to me. Thanks for simplifying the latter!

I would probably prefer to use the upstream fix, if easily possible, rather than the workaround with weaker CFLAGS.

comment:46 Changed 17 months ago by John Palmieri

The upstream fix will work for me (if it applies to 1.20.3 — I haven't tested yet), but I don't know if it will work for the issue cited in the ticket description.

Last edited 17 months ago by John Palmieri (previous) (diff)

comment:47 Changed 17 months ago by gh-kliem

The patch from comment:18 might work for this. As we have a working branch, the reasonable thing would be to open another ticket that tries to apply both patches to fix the problem?

comment:48 Changed 17 months ago by git

Commit: 9c87b6571ec5bb9a4dce5f2761906b0110e2709a56ffafb6af3cba4d7d203962ffa772fc9366f9c1

Branch pushed to git repo; I updated commit sha1. New commits:

56ffafbmore consistent header guard name

comment:49 Changed 17 months ago by John Palmieri

The upstream patch applies and Sage builds with it. I like the idea of opening another ticket with the two patches and also attempting to remove export CFLAGS="$CFLAGS_NON_NATIVE".

comment:50 in reply to:  49 Changed 17 months ago by John Palmieri

Replying to jhpalmieri:

The upstream patch applies and Sage builds with it. I like the idea of opening another ticket with the two patches and also attempting to remove export CFLAGS="$CFLAGS_NON_NATIVE".

See #32080.

comment:51 Changed 17 months ago by git

Commit: 56ffafb6af3cba4d7d203962ffa772fc9366f9c1a7eef1b2357cd3862facf64edb81abdafbda6923

Branch pushed to git repo; I updated commit sha1. New commits:

f02780f.github/workflows/ci-cygwin-*: Repair passing '--enable-fat-binary' to configure
a7eef1bMerge tag '9.4.beta4' into t/32021/public/32021

comment:52 Changed 17 months ago by Matthias Köppe

I've pushed a fix for the Cygwin CI script (see #32080).

comment:53 Changed 17 months ago by Matthias Köppe

Reviewers: https://github.com/mkoeppe/sage/actions/runs/1005406512

comment:54 Changed 17 months ago by John Palmieri

Reviewers: https://github.com/mkoeppe/sage/actions/runs/1005406512John Palmieri, https://github.com/mkoeppe/sage/actions/runs/1005406512, ...

It's all fine with me. I am willing to trust Matthias' bot for the Cygwin stuff, but I would be happier if someone else could verify the problem and solution. I've taken a look at the changes to sage-build-env and sage-build-env-config.in and they make sense, but again, I would like more people to look at it.

Given those caveats, positive review from me.

comment:55 Changed 17 months ago by Dima Pasechnik

probably this should be tested with a pynac fix for Cygwin - otherwise there are errors installing pynac.

comment:56 Changed 17 months ago by Matthias Köppe

Reviewers: John Palmieri, https://github.com/mkoeppe/sage/actions/runs/1005406512, ...John Palmieri, Matthias Koeppe
Status: needs_reviewpositive_review

Looks good enough for me on Cygwin. The pynac problem is unrelated.

Let's get this in.

comment:57 Changed 17 months ago by Volker Braun

Branch: public/32021a7eef1b2357cd3862facf64edb81abdafbda6923
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.