Opened 18 months ago

Last modified 6 days ago

#27122 needs_review enhancement

Compile with -march=native

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.2
Component: build Keywords: Intrinsics, SIMD
Cc: selia, vbraun, embray, fbissey Merged in:
Authors: Jonathan Kliem Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: u/gh-kliem/march_native_in_configure (Commits) Commit: e9fb1a37af6ac08c29dff5a9cb062eccf60f761a
Dependencies: Stopgaps:

Description (last modified by gh-kliem)

We should compile Sage with the extra compile argument -march=native for extra performance, at least if SAGE_FAT_BINARY is not set.

In gcc/spkg_configure.ac we check, whether the argument -march=native is accepted.

In build/make/Makefile.in we assemble the flags.

We also gather and clean up other compiler flags. In short we

  • add --enable-fat-binary to configure (as an alternative to setting SAGE_FAT_BINARY)
  • add --enable_debug={no|symbols|yes} as alternative to setting SAGE_DEBUG
  • compile with -Og -g if SAGE_DEBUG=yes
  • compile with -O2 if SAGE_DEBUG=no (some packages with O3 instead)
  • compile with -O2 -g otherwise (some packages with O3 instead)

We add -march=native to the later two, if not building fat binaries and the compiler and the package accepts it.

During the build of sage there are now flags that can be exported in spkg_install.in with one line, instead of checking SAGE_DEBUG etc. for each individual package, e.g. by

  • export CFLAGS=$CFLAGS (redundant, use flags as above with -march=native if possible)
  • export CFLAGS=$CFLAGS_NON_NATIVE (flags as above without -march=native)
  • export CFLAGS=$CFLAGS_O3 (O3 instead of O2, but only if not in debug mode, add -march=native if possible)
  • export CFLAGS=$CFLAGS_O3_NON_NATIVE (O3 instead of O2, no -march=native)
  • export CFLAGS=$ORIGINAL_CFLAGS (use $CFLAGS as set before configure)

Likewise we do with CXXFLAGS, FCFLAGS, F77FLAGS.

We try to respect the users choice and do not overwrite the flags if already set by the user, but only add compile flags if absolutely necessary for packages to work.

Inidividual packages can still be compiled with specified flags or in debug mode using

make CFLAGS=-O2 some-package

or

make SAGE_DEBUG="yes" some-package

Change History (123)

comment:1 Changed 18 months ago by jdemeyer

  • Description modified (diff)
  • Keywords Polyhedra FindPoly removed
  • Summary changed from Adding -march=native to build/cythonized/setup.py to Compile with -march=native

The topic has nothing to do with polyhedra, many parts of Sage could benefit from it.

comment:2 Changed 18 months ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 16 months ago by embray

  • Milestone changed from sage-8.7 to sage-8.8

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

comment:4 Changed 13 months ago by embray

  • Milestone sage-8.8 deleted

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

comment:5 Changed 9 months ago by gh-kliem

  • Priority changed from minor to major

comment:6 Changed 9 months ago by gh-kliem

  • Branch set to public/27122
  • Commit set to cdc86563cfe52a78e9e50b1ce374750434e1dc1e

New commits:

cdc8656compiling with `-march=native` for GCC from 5.1 and clang from 6.0 if SAGE_FAT_BINARY is not set

comment:7 Changed 9 months ago by gh-kliem

  • Status changed from new to needs_review

I added a very naive approach of doing it. Wondering if this is remotely ok or going in the right direction.

comment:8 Changed 9 months ago by dimpase

looks OK - not sure whether converting version to float() is normal practice, but that's minor.

I'm also not sure whether this will pass the argument to Cython, or it needs

# distutils: extra_compile_args = -march=native

in *.pxd files.

comment:9 Changed 9 months ago by gh-kliem

I believe it works, but I'm not entirely sure.

This seems to state that sage/stats/distributions/discrete_gaussian_integer.pyx was compiled with -march=native.

I have not set it with CFLAGS, in that case it would have appeared twice on the list.

[sagelib-9.0.beta2] [469/499] gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -fPIC -I./sage/cpython -I/srv/public/kliem/sage/local/lib/python2.7/site-packages/cypari2 -I./sage/libs/ntl -I/srv/public/kliem/sage/local/lib/python2.7/site-packages/
cysignals -I/srv/public/kliem/sage/local/include -I/srv/public/kliem/sage/src -I/srv/public/kliem/sage/src/sage/ext -I/srv/public/kliem/sage/local/include/python2.7 -I/srv/public/kliem/sage/local/lib/python2.7/site-packages/numpy/core/include -Ibuild/cythonized -I/srv/pu
blic/kliem/sage/local/include/python2.7 -c build/cythonized/sage/stats/distributions/discrete_gaussian_integer.c -o build/temp.linux-x86_64-2.7/build/cythonized/sage/stats/distributions/discrete_gaussian_integer.o -fno-strict-aliasing -DCYTHON_CLINE_IN_TRACEBACK=1 -march
=native -D_XOPEN_SOURCE=600 -std=c99                                                                                                                                                                                                                                           
[sagelib-9.0.beta2] build/cythonized/sage/stats/hmm/hmm.c: In function ‘__pyx_pw_4sage_5stats_3hmm_3hmm_25DiscreteHiddenMarkovModel_17_forward’:          

comment:10 Changed 9 months ago by gh-kliem

  • Description modified (diff)

It works for me. All tests passed. No significant time difference.

  • Operating System: Debian GNU/Linux 10 (buster)
  • Kernel: Linux 4.19.0-6-amd64
  • Architecture: x86-64
  • CPU Model name: Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz
Last edited 9 months ago by gh-kliem (previous) (diff)

comment:11 Changed 9 months ago by gh-kliem

  • Description modified (diff)

comment:12 Changed 9 months ago by gh-kliem

  • Cc selia added

comment:13 Changed 9 months ago by gh-kliem

Works on

  • Operating System: Ubuntu 18.04.3 LTS
  • Kernel: Linux 4.15.0-65-generic
  • Architecture: x86-64
  • Model name: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz

comment:14 Changed 9 months ago by charpent

Success on my notebook running a Python 3-based 9.0.beta2 on top of Debian testing :

  • Operating System: Debian GNU/Linux bullseye/sid
  • Kernel: Linux 5.2.0-3-amd64
  • Architecture: x86-64
  • Nom de modèle : Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz

HTH,

Last edited 9 months ago by charpent (previous) (diff)

comment:15 Changed 9 months ago by dimpase

works on Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz (in 32-bit mode), Ubunty 14.04 LTS

comment:16 Changed 9 months ago by mjo

I don't think we should be reinventing the thirty-year-old standard CFLAGS here. It looks like setup.py will already use CFLAGS if it's set. I'm not arguing against sane defaults, just saying that appending -march=native on top of my CFLAGS is a bit rude if I've set them to what I want.

Instead, why don't we try to set a decent default for CFLAGS if the user does not already have it set? Typically this amounts to something like

CFLAGS ?= -march=native -O2

in a makefile somewhere. For people who don't care, CFLAGS will get set to something nice, and setup.py will use that value. For the people who have set CFLAGS to something special, nothing goes wrong.

comment:17 Changed 9 months ago by git

  • Commit changed from cdc86563cfe52a78e9e50b1ce374750434e1dc1e to 9aabee1ecf8507cf68a6122ca3febfa0416af528

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

9aabee1respect CFLAGS

comment:18 follow-up: Changed 9 months ago by gh-kliem

Ok, this should respect CFLAGS now.

I need to check yet, that the compiler can handle march=native. This is why I did it in the setup file.

comment:19 in reply to: ↑ 18 ; follow-up: Changed 9 months ago by mjo

Replying to gh-kliem:

Ok, this should respect CFLAGS now.

I need to check yet, that the compiler can handle march=native. This is why I did it in the setup file.

I'd bet that our ./configure script is capable of doing that in a more reliable way, since it's something that C programs need to do often and python programs basically never. For example, I think we already check for a suitable version of gcc to see if we need to build our own. And we include an autoconf macro (m4/ax_c_check_flag.m4) that will try to compile a file using -march=native, and then report back if it worked or not.

Ideally, CFLAGS set in the environment will work everywhere, subject to upstream/spkg cooperation. But they definitely work in setup.py, so if we're able to detect these features at a higher level and then (by default) set CFLAGS/CXXFLAGS to include them, more code would be able to benefit from it. The setup.py script would still pick them up from the environment, but anything else that respects the environment variables would benefit, too.

(I don't know off the top of my head that setup.py doesn't affect spkgs, but I would guess not.)

comment:20 in reply to: ↑ 19 Changed 8 months ago by gh-kliem

Thanks for the comment by the way. It makes sense to me.

I'm just very busy at the moment and didn't have time to see how that could be handled by the configure script.

Replying to mjo:

Replying to gh-kliem:

Ok, this should respect CFLAGS now.

I need to check yet, that the compiler can handle march=native. This is why I did it in the setup file.

I'd bet that our ./configure script is capable of doing that in a more reliable way, since it's something that C programs need to do often and python programs basically never. For example, I think we already check for a suitable version of gcc to see if we need to build our own. And we include an autoconf macro (m4/ax_c_check_flag.m4) that will try to compile a file using -march=native, and then report back if it worked or not.

Ideally, CFLAGS set in the environment will work everywhere, subject to upstream/spkg cooperation. But they definitely work in setup.py, so if we're able to detect these features at a higher level and then (by default) set CFLAGS/CXXFLAGS to include them, more code would be able to benefit from it. The setup.py script would still pick them up from the environment, but anything else that respects the environment variables would benefit, too.

(I don't know off the top of my head that setup.py doesn't affect spkgs, but I would guess not.)

comment:21 follow-up: Changed 6 months ago by mjo

This does the trick, but might be a bit blunt:

diff --git a/build/make/Makefile.in b/build/make/Makefile.in
index eea7ceb7dc..a5966b8011 100644
--- a/build/make/Makefile.in
+++ b/build/make/Makefile.in
@@ -17,6 +17,13 @@
 # Always use bash for make rules
 SHELL = @SHELL@

+# Use safe-but-efficient optimization flags, if the user does not
+# provide his own.
+CFLAGS ?= -O2 -march=native
+CXXFLAGS ?= -O2 -march=native
+export CFLAGS
+export CXXFLAGS
+
 ifndef SAGE_SPKG_INST
 ifndef DEBUG_RULES
 $(error This Makefile needs to be invoked by build/make/install)

In any case, it would make sense at that point to go through every spkg-install and strip out any "-O2" and "-march=native" additions.

comment:22 Changed 6 months ago by gh-kliem

  • Branch changed from public/27122 to public/27122-new
  • Commit changed from 9aabee1ecf8507cf68a6122ca3febfa0416af528 to 60bddf468a3c0b0e70ec285ebf450ca45eabf8f3

New commits:

b66e81dcompile with '-O2 -march=native' if the user does not provide own flags
e16ba0eadded '-g' as standard compile flag; '-O0' is standard in case of debugging now
11b9175much simpler by using testcflags.sh
a574572removed redundant flags -O2, -O0, -g
60bddf4remove flag -Wall by default

comment:23 in reply to: ↑ 21 Changed 6 months ago by gh-kliem

Thanks. I basically did it like this.

I also added -O0, -O2 and -g according to SAGE_DEBUG. This helps clean up a lot of code (basically every package sets those flags manually).

Finally, I used src/bin/testcflags.sh to remove unsupported flags.

The package ecm still adds -march=native on top of CFLAGS unless a similar flag is present.

Some packages prefer -O3 over -O2, so I left the flag -O3 as it is.

With this branch all tests passed on my end (ran distclean and installed all packages I touched).

Replying to mjo:

This does the trick, but might be a bit blunt:

diff --git a/build/make/Makefile.in b/build/make/Makefile.in
index eea7ceb7dc..a5966b8011 100644
--- a/build/make/Makefile.in
+++ b/build/make/Makefile.in
@@ -17,6 +17,13 @@
 # Always use bash for make rules
 SHELL = @SHELL@

+# Use safe-but-efficient optimization flags, if the user does not
+# provide his own.
+CFLAGS ?= -O2 -march=native
+CXXFLAGS ?= -O2 -march=native
+export CFLAGS
+export CXXFLAGS
+
 ifndef SAGE_SPKG_INST
 ifndef DEBUG_RULES
 $(error This Makefile needs to be invoked by build/make/install)

In any case, it would make sense at that point to go through every spkg-install and strip out any "-O2" and "-march=native" additions.

comment:24 Changed 6 months ago by gh-kliem

Is the new code acceptable?

comment:25 Changed 6 months ago by mjo

LGTM at first glance. Good job figuring out what to do with SAGE_DEBUG and SAGE_FAT_BINARY, I missed those entirely.

I've never tried passing -march=native to gfortran, but it looks like it should work. I've added it to my local config for future builds.

The use of testcflags.sh only attempts to compile a C program with those flags, but then appends them to the compiler flags for C++ and fortran as well. That's not 100% correct, but I think we're already making the assumption that every compiler is from the same version of the same gcc suite? (Anyone?) So long as it works, I'd be happy with a comment about that. In the future, we could always change it to use the AX_CHECK_COMPILE_FLAG repeatedly, but that's overkill until we can actually swap out compilers.

comment:26 Changed 6 months ago by gh-kliem

  • Branch changed from public/27122-new to public/27122-reb
  • Commit changed from 60bddf468a3c0b0e70ec285ebf450ca45eabf8f3 to e4205465daa4267c12a3ff49d23471b277bc16d5

Ok, I added a comment about that assumption.

Is this at a point where people can/should test it again?


New commits:

0045dbacompile with '-O2 -march=native' if the user does not provide own flags
4241dfeadded '-g' as standard compile flag; '-O0' is standard in case of debugging now
ea3c0dfmuch simpler by using testcflags.sh
60e3c36removed redundant flags -O2, -O0, -g
bec54ccremove flag -Wall by default
e420546adding comment about assuming compilers from same suite

comment:27 Changed 6 months ago by gh-kliem

  • Authors set to Jonathan Kliem

comment:28 Changed 6 months ago by mjo

Yeah, now it just needs testing on a few different platforms. I've been using -march=native in my exported CFLAGS for years without problems, and your implementation looks OK to me, but I'm on a boring amd64/linux system and these days I'm doing my best to *not* build any spkgs.

While testing, you should be sure that the spkg really gets used (otherwise, the CFLAGS are irrelevant). The ./configure script nowadays will do its best to use packages from the system instead of an spkg. You can disable that using e.g. --without-system-foo, but there are a lot of flags you have to pass. Something like

$ ./configure --help | grep -oP '\-\-with-system[^ =]+' | sort | uniq | sed 's/with/without/'
--without-system-arb
--without-system-atlas
--without-system-bzip2
...

can be used to get the full list for your branch.

comment:29 Changed 6 months ago by gh-kliem

Ok. Took a while, but it seems to work now. The test sage -t --long src/sage/interfaces/psage.py only worked on retry, but I guess this is fine.

What I did was basically the following:

$ make distclean
$ ./configure $(./configure --help | grep -oP '\-\-with-system[^ =]+' | sort | uniq | sed 's/with/without/' | grep -v gfortran | past -sd " ")
$ make build; ./sage -i openssl; ./sage -f python3; ./sage -i pyopenssl; make build
$ ./sage -i $(./sage --optional | awk -F\. '{print $1}' | grep -v warnings | grep -v package |  grep -v gfort | grep -v buckygen |grep -v database | grep -v polymake | grep -v jupymake | paste -sd " ")
$ sage -i sage_numerical_backends_coin
$ make
$ sage -t --long --all

(install gcc)

$ make distclean
$ ./configure $(./configure --help | grep -oP '\-\-with-system[^ =]+' | sort | uniq | sed 's/with/without/' | grep -v gfortran | grep -v gcc | past -sd " ")
$ make build; ./sage -i openssl; ./sage -f python3; ./sage -i pyopenssl; make build
$ ./sage -i $(./sage --optional | awk -F\. '{print $1}' | grep -v warnings | grep -v package | grep -v gfort | grep -v buckygen |grep -v database | grep -v polymake | grep -v jupymake | paste -sd " ")
$ sage -i sage_numerical_backends_coin; make; sage -t --long --all

(use system gcc)

I'm often having trouble with openssl and I don't exactly understand the problem (but the workaround seems to work), so I guess you can ignore that part.

I would suggest the following for others to test it:

  • Pull ticket.
  • Unset CFLAGS etc.
  • make distclean
  • configure to compile as you would usually do, plus add the following:
    $ ./configure --help | grep -oP '\-\-with-system[^ =]+' | sort | uniq | sed 's/with/without/' | grep -v gfortran | grep -v gcc | past -sd " "
    
    so e.g. one would configure to compile with clang as follows:
    $ CC=clang CXX=clang++ FC=flang ./configure $(./configure --help | grep -oP '\-\-with-system[^ =]+' | sort | uniq | sed 's/with/without/' | grep -v gfortran | grep -v gcc | past -sd " ")
    
  • make sage
  • install optional packages
    $ ./sage -i $(./sage --optional | awk -F\. '{print $1}' | grep -v warnings | grep -v package |  grep -v gfort | grep -v buckygen |grep -v database | grep -v polymake | grep -v jupymake | paste -sd " ") sage_numerical_backends_coin
    
  • test everything with sage -t --all --long

Report any (unusual) trouble you are having.

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

comment:30 follow-up: Changed 5 months ago by mkoeppe

To test build changes like this systematically, I recommend to use #29053 / #29087.

comment:31 in reply to: ↑ 30 Changed 5 months ago by gh-kliem

https://github.com/kliem/sage-test-27122/runs/422709282

I pulled the newest develop and #29087 to run those tests.

It's really hard to see the difference to your run: https://github.com/mkoeppe/sage/runs/422167181. I'm reducing my attention here to those tests that suceeded for you:

  • Problems with gsl for
    • ubuntu-xenial, minimal, ubuntu-latest
    • ubuntu-xenial, standard, ubuntu-latest
  • Problems with gsl and libatomic_ops for
    • ubuntu-bionic, standard, ubuntu-latest
    • ubuntu-eoan, standard, ubuntu-latest
    • fedora-26, standard, ubuntu-latest
    • fedora-28, standard, ubuntu-latest
  • Never ran the following:
    • ubuntu-bionic-i386, standard:

From this I gather that maybe one shouldn't use -march=native for those two packages. Does this make any sense?

Replying to mkoeppe:

To test build changes like this systematically, I recommend to use #29053 / #29087.

comment:32 Changed 3 months ago by gh-kliem

  • Branch changed from public/27122-reb to public/27122-reb2
  • Commit changed from e4205465daa4267c12a3ff49d23471b277bc16d5 to 301bd2c279991115c5702b1837118339360e8e86

New commits:

dc0e744compile with '-O2 -march=native' if the user does not provide own flags
f8ce3b3added '-g' as standard compile flag; '-O0' is standard in case of debugging now
f645f71much simpler by using testcflags.sh
69b6d35removed redundant flags -O2, -O0, -g
81a6ea5remove flag -Wall by default
301bd2cadding comment about assuming compilers from same suite

comment:33 follow-up: Changed 3 months ago by gh-kliem

Can someone please help me with the following issue.

  • conda-forge and homebrew-macos
      `/bin/sh: ../../src/bin/./testcflags.sh: No such file or directory`
    
    The following line seems to be a problem:
    +opt := $(shell export CC=$(CC) && ../../src/bin/./testcflags.sh $(opt))
    

Otherwise things are looking good, I would say.

Things have improved "by themselves" (of course not):

https://github.com/kliem/sage-test-27122/actions/runs/66307817

Now the current ticket (with #29087 merged) runs as smooth as the current beta https://github.com/mkoeppe/sage/actions/runs/65613035. However many tests got cancelled due to exceeding 6 hours.

  • ubuntu-trusty, minimal:
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/matrix/matrix2.pyx # Timed out
      sage: all(L.change_ring(SR).is_cross_positive_on(K)
          for L in K.cross_positive_operators_gens())  # long time ## line 15240 ##
      

Got cancelled at sage -t src/sage/numerical/linear_tensor.py (6 hours)

  • ubuntu-trusty, standard
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out

Got cancelled at sage -t src/sage/modular/local_comp/liftings.py

Got cancelled at sage -t src/sage/schemes/affine/affine_subscheme.py

  • ubuntu-xenial, minimal -- ubuntu-bionic, minimal -- debian-jessie, minimal -- debian-buster, minimal -- fedora-26, minimal -- fedora-26, standard -- centos-8, standard -- archlinux-latest, minimal
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed
  • ubuntu-xenial, standard -- ubuntu-xenial, standard-python2
    • sage -t src/sage/graphs/digraph_generators.py # 5 doctests failed
    • sage -t src/sage/interfaces/tachyon.py # 1 doctest failed
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed
    • sage -t src/sage/tests/cmdline.py # 3 doctests failed
  • ubuntu-bionic, standard -- ubuntu-bionic, standard-python2 -- ubuntu-focal, standard -- ubuntu-focal, standard-python2 -- linuxmint-19.3, standard, linuxmint-19.3, standard-python2
    • sage -t src/sage/interfaces/r.py # 1 doctest failed
    • sage -t src/sage/interfaces/tachyon.py # 1 doctest failed
    • sage -t src/sage/libs/glpk/error.pyx # 1 doctest failed
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/numerical/backends/glpk_backend.pyx # 1 doctest failed
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed (only python3)
    • sage -t src/sage/tests/cmdline.py # 3 doctests failed
  • ubuntu-eoan, minimal Got cancelled at sage -t src/sage/manifolds/differentiable/differentiable_submanifold.py
  • ubuntu-eoan, standard
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/u/underscore/libjs-underscore_1.8.3~dfsg-1_all.deb  Could not connect to azure.archive.ubuntu.com:80 (52.177.174.250), connection timed out
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/s/sphinx/libjs-sphinxdoc_1.6.7-1ubuntu1_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/p/python-pluggy/python3-pluggy_0.6.0-1_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/p/python-py/python3-py_1.5.2-1_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/p/python-setuptools/python3-setuptools_39.0.1-2_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/p/python-virtualenv/python3-virtualenv_15.1.0+ds-1.1_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/p/python-virtualenv/virtualenv_15.1.0+ds-1.1_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/t/tox/tox_2.5.0-1_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/t/tox/python-tox_2.5.0-1_all.deb  Unable to connect to azure.archive.ubuntu.com:http:
    E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
    
  • ubuntu-eoan, standard-python2
    • sage -t src/sage/interfaces/r.py # 1 doctest failed
    • sage -t src/sage/interfaces/tachyon.py # 1 doctest failed
    • sage -t src/sage/libs/eclib/interface.py # 1 doctest failed
    • sage -t src/sage/libs/glpk/error.pyx # 1 doctest failed
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/numerical/backends/glpk_backend.pyx # 1 doctest failed
    • sage -t src/sage/tests/cmdline.py # 3 doctests failed
  • ubuntu-focal, minimal

cancelled at sage -t src/sage/geometry/hyperplane_arrangement/check_freeness.py

  • debian-jessie, standard
    • sage -t src/sage/doctest/test.py # Timed out
       sage: if system() == "Linux":
          P = subprocess.Popen(["sage", "-t", "--warn-long", "0", "--memlimit=2000", "memlimit.rst"], stdout=subprocess.PIPE, **kwds)
          out, err = P.communicate()
          ok = ("MemoryError: failed to allocate" in bytes_to_str(out)) ## line 521 ##
      
      probably related to https://groups.google.com/d/msg/sage-release/kU5M1QVuQQY/IEX-j4PDAwAJ
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed
    • sage -t src/sage/tests/cmdline.py # 3 doctests failed
  • debian-jessie, standard-python2
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py Timed out
    • sage -t src/sage/doctest/test.py # 1 doctest failed cancelled at sage -t src/sage/rings/rational.pxd
  • debian-buster, standard -- debian-buster, standard-python2
    • sage -t src/sage/interfaces/r.py # 1 doctest failed
    • sage -t src/sage/interfaces/tachyon.py # 1 doctest failed
    • sage -t src/sage/libs/glpk/error.pyx # 1 doctest failed
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/numerical/backends/glpk_backend.pyx # 1 doctest failed
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed (only python3)
    • sage -t src/sage/schemes/hyperelliptic_curves/hyperelliptic_finite_field.py # 2 doctests failed
    • sage -t src/sage/tests/cmdline.py # 3 doctests failed
  • debian-bullseye, minimal
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out cancelled at sage -t src/sage/modular/modform/weight1.py
  • debian-bullseye, standard -- debian-bullseye, standard-python2 -- debian-sid, standard
    • sage -t src/sage/interfaces/r.py # 1 doctest failed
    • sage -t src/sage/interfaces/tachyon.py # 1 doctest failed
    • sage -t src/sage/lfunctions/dokchitser.py # 2 doctests failed
    • sage -t src/sage/lfunctions/pari.py # 1 doctest failed
    • sage -t src/sage/libs/glpk/error.pyx # 1 doctest failed
    • sage -t src/sage/interfaces/maxima.py # Timed out (only debian-sid, standard-python2)
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/numerical/backends/glpk_backend.pyx # 1 doctest failed
    • sage -t src/sage/rings/number_field/number_field_ideal.py # 2 doctests failed
    • sage -t src/sage/rings/number_field/number_field.py # 8 doctests failed
    • sage -t src/sage/rings/number_field/unit_group.py # 1 doctest failed
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed (python3 only)
    • sage -t src/sage/rings/polynomial/polynomial_quotient_ring.py # 2 doctests failed
    • sage -t src/sage/rings/number_field/number_field_element.pyx # Timed out
    • sage -t src/sage/schemes/elliptic_curves/ell_number_field.py # 3 doctests failed
    • sage -t src/sage/schemes/elliptic_curves/height.py # 6 doctests failed
    • sage -t src/sage/schemes/plane_conics/con_number_field.py # 1 doctest failed
    • sage -t src/sage/tests/cmdline.py # 3 doctests failed
  • debian-sid, minimal

cancelled at sage -t src/sage/misc/bindable_class.py

  • linuxmint-19.3, minimal
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed cancelled at sage -t src/sage/rings/polynomial/polynomial_number_field.pyx
  • fedora-26, standard-python2
  • fedora-28, minimal

cancelled at sage -t src/sage/plot/disk.py

  • centos-7, minimal
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out cancelled at sage -t src/sage/sat/solvers/satsolver.pxd
  • centos-8, standard

cancelled during [pplpy-0.8.4] installing. Log file: /sage/logs/pkgs/pplpy-0.8.4.log

  • centos-8, standard-python2
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
  • centos-8, minimal

cancelled at sage -t src/sage/manifolds/differentiable/characteristic_class.py

  • centos-8, standard-python2

cancelled during traceback of sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out

  • archlinux-latest, standard -- archlinux-latest, standard-python2
    • sage -t src/sage/libs/glpk/error.pyx # 1 doctest failed
    • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
    • sage -t src/sage/numerical/backends/glpk_backend.pyx # 1 doctest failed
    • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed (python3 only)
  • slackware (build failure, see #29373)

comment:34 in reply to: ↑ 33 ; follow-up: Changed 3 months ago by gh-kliem

  • Status changed from needs_review to needs_info

Replying to gh-kliem:

Can someone please help me with the following issue.

  • conda-forge and homebrew-macos
      `/bin/sh: ../../src/bin/./testcflags.sh: No such file or directory`
    
    The following line seems to be a problem:
    +opt := $(shell export CC=$(CC) && ../../src/bin/./testcflags.sh $(opt))
    

comment:35 in reply to: ↑ 34 Changed 3 months ago by mkoeppe

Replying to gh-kliem:

Replying to gh-kliem:

Can someone please help me with the following issue.

  • conda-forge and homebrew-macos
      `/bin/sh: ../../src/bin/./testcflags.sh: No such file or directory`
    
    The following line seems to be a problem:
    +opt := $(shell export CC=$(CC) && ../../src/bin/./testcflags.sh $(opt))
    

Where do you see this line?

comment:36 follow-up: Changed 3 months ago by mkoeppe

Oh, I see, you added this on the ticket. See #29381.

comment:37 Changed 3 months ago by mkoeppe

Lots of the doctest failures that you reported can also be seen on 9.1.beta9 on these platforms. Help with fixing them is very welcome. See https://groups.google.com/d/msg/sage-release/kU5M1QVuQQY/5vDLmipuAwAJ

comment:38 Changed 3 months ago by git

  • Commit changed from 301bd2c279991115c5702b1837118339360e8e86 to 2d1d7980c413afc1e5c6625c16b3bac99daa7463

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

2d1d798testcflags.sh has moved to build/bin

comment:39 in reply to: ↑ 36 Changed 3 months ago by gh-kliem

Replying to mkoeppe:

Oh, I see, you added this on the ticket. See #29381.

Thanks.

Well, turns out I haven't tested anything new. Enabling march=native just failed everywhere and I retested the newest beta.

comment:40 Changed 3 months ago by gh-kliem

  • Status changed from needs_info to needs_review

Seems to work now:

https://github.com/kliem/sage-test-27122/actions/runs/67557444

Some strange fail in (debian-bullseye, standard):

cp: cannot create directory '/sage/local/./lib/python3.7/site-packages/Cython/__pycache__': File exists

Looks somewhat like a racing condition to me. Given that it bullseye suceeds on minimal and standard-python2, I would say it's fine.

Otherwise its the usual suspects (note that macos got cancelled due to timout):

  • sage -t src/sage/doctest/test.py # 1 doctest failed
  • sage -t src/sage/graphs/digraph_generators.py # 5 doctests failed

e.g. ubuntu-xenial, standard, might be related to #28958

seems to use system nauty: configure: will use system package and not install SPKG nauty

  • sage -t src/sage/interfaces/r.py # 1 doctest failed #29471
  • sage -t src/sage/interfaces/tachyon.py # 1 doctest failed #29442
  • sage -t src/sage/lfunctions/dokchitser.py # 2 doctests failed #29472
  • sage -t src/sage/lfunctions/pari.py # 1 doctest failed #29472
  • sage -t src/sage/libs/eclib/interface.py # 2 doctests failed #28943
  • sage -t src/sage/libs/glpk/error.pyx # 1 doctest failed #29493
  • sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out #29440
  • sage -t src/sage/numerical/backends/glpk_backend.pyx # 1 doctest failed #29493
  • sage -t src/sage/rings/number_field/unit_group.py # 1 doctest failed #29472
  • sage -t src/sage/rings/number_field/number_field_element.pyx # Timed out #29472
  • sage -t src/sage/rings/number_field/number_field_ideal.py # 2 doctests failed #29445 and #29472
  • sage -t src/sage/rings/number_field/number_field.py # 8 doctests failed #29472
  • sage -t src/sage/rings/padics/padic_lattice_element.py # 3 doctests failed #29272
  • sage -t src/sage/rings/polynomial/polynomial_quotient_ring.py # 2 doctests failed #29472
  • sage -t src/sage/schemes/elliptic_curves/ell_number_field.py # 3 doctests failed #29472 (fixes 2 of them)
  • sage -t src/sage/schemes/elliptic_curves/height.py # 6 doctests failed
  • sage -t src/sage/schemes/plane_conics/con_number_field.py # 1 doctest failed
  • sage -t src/sage/schemes/hyperelliptic_curves/hyperelliptic_finite_field.py # 2 doctests failed #29494
  • sage -t src/sage/tests/cmdline.py # 3 doctests failed #29092
Last edited 3 months ago by gh-kliem (previous) (diff)

comment:41 Changed 3 months ago by gh-kliem

  • Description modified (diff)

comment:42 Changed 3 months ago by gh-kliem

  • Milestone set to sage-9.2

Is this ticket anywhere near a positive review?

It would be nice to migrate my own implementation for bitsets for CombinatorialPolyhedron to the one in data_structures/bitsets.pxi. That would prevent a lot of code duplication for #29191. But I would only do this, if I can modify data_structures/bitsets.pxi such that it uses intrinsics.

But, enabling intrinsics without -march=native as default would mean that a lot of code is never really tested as many cases are very dependent on the specific architecture (see #27103).

comment:43 Changed 3 months ago by dimpase

One problem with -march=native as default is that building binary packages with it is a bit hard (unless it is combined with settings for a low grade CPU).

Perhaps, keeping it a non-default is more meaningful?

comment:44 Changed 3 months ago by gh-kliem

Isn't that what SAGE_FAT_BINARY is for?

I just noticed that need to rebase.

comment:45 Changed 3 months ago by gh-kliem

  • Branch changed from public/27122-reb2 to public/27122-reb3
  • Commit changed from 2d1d7980c413afc1e5c6625c16b3bac99daa7463 to bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0

New commits:

24c1a87compile with '-O2 -march=native' if the user does not provide own flags
5ec3142added '-g' as standard compile flag; '-O0' is standard in case of debugging now
8ca0b48much simpler by using testcflags.sh
ecb7606removed redundant flags -O2, -O0, -g
5bdf4e4remove flag -Wall by default
4543249adding comment about assuming compilers from same suite
bc6f7c1testcflags.sh has moved to build/bin

comment:46 Changed 3 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

Looks good to me and should be merged early in the 9.2 series

comment:47 Changed 3 months ago by gh-kliem

Thanks.

comment:48 Changed 3 months ago by mkoeppe

Looks like @vbraun is already merging it for 9.1.

In a test run for ubuntu-trusty-minimal (https://github.com/mkoeppe/sage/runs/617198833) I see a new error building gfortran:

/sage/local/var/tmp/sage/build/gfortran-9.2.0/gcc-build/./gcc/xgcc -B/sage/local/var/tmp/sage/build/gfortran-9.2.0/gcc-build/./gcc/ -B/sage/local/x86_64-pc-linux-gnu/bin/ -B/sage/local/x86_64-pc-linux-gnu/lib/ -isystem /sage/local/x86_64-pc-linux-gnu/include -isystem /sage/local/x86_64-pc-linux-gnu/sys-include    -O2 -g -march=native -O2  -O2 -g -march=native -DIN_GCC    -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include   -fpic -mlong-double-80 -DUSE_ELF_SYMVER  -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector   -fpic -mlong-double-80 -DUSE_ELF_SYMVER  -I. -I. -I../.././gcc -I../../../src/libgcc -I../../../src/libgcc/. -I../../../src/libgcc/../gcc -I../../../src/libgcc/../include -I../../../src/libgcc/config/libbid -DENABLE_DECIMAL_BID_FORMAT -DHAVE_CC_TLS  -DUSE_TLS -o _ffssi2_s.o -MT _ffssi2_s.o -MD -MP -MF _ffssi2_s.dep -DSHARED -DL_ffssi2 -c ../../../src/libgcc/libgcc2.c
/tmp/ccjkcyn0.s: Assembler messages:
/tmp/ccjkcyn0.s:3937: Error: no such instruction: `vmovdqu8 (%rcx),%xmm0'
/tmp/ccjkcyn0.s:3939: Error: no such instruction: `vmovdqu8 -16(%rcx,%r8),%xmm1'
/tmp/ccjkcyn0.s:6032: Error: operand size mismatch for `vmovdqa64'
/tmp/ccjkcyn0.s:6033: Error: operand size mismatch for `vmovdqa64'
/tmp/ccjkcyn0.s:6034: Error: operand size mismatch for `vmovdqa64'
/tmp/ccjkcyn0.s:6035: Error: operand size mismatch for `vmovdqa64'
/tmp/ccjkcyn0.s:6036: Error: operand size mismatch for `vmovdqa64'
/tmp/ccjkcyn0.s:6037: Error: operand size mismatch for `vmovdqa64'

Also #29575 (debian-jessie: fflas_ffpack again) is from that run.

So this may need some more work

comment:49 Changed 3 months ago by mkoeppe

  • Status changed from positive_review to needs_work

comment:50 Changed 3 months ago by mkoeppe

Also segfaults building dochtml with ubuntu-bionic-standard (https://github.com/mkoeppe/sage/runs/617198861)

comment:51 Changed 3 months ago by mkoeppe

  • Cc vbraun added

comment:52 follow-up: Changed 3 months ago by mkoeppe

Also segfault on linuxmint-19.3-minimal (https://github.com/mkoeppe/sage/runs/617198994)

Definitely not ready for 9.1.

comment:53 Changed 3 months ago by mkoeppe

Also breaks gfortran build on cygwin-minimal (https://github.com/mkoeppe/sage/runs/617198553)

comment:54 Changed 3 months ago by vbraun

  • Branch changed from public/27122-reb3 to bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0
  • Resolution set to fixed
  • Status changed from needs_work to closed

comment:55 Changed 3 months ago by vbraun

  • Commit bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

comment:56 Changed 3 months ago by gh-kliem

  • Branch changed from bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0 to public/27122-reb3
  • Commit set to bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0

New commits:

24c1a87compile with '-O2 -march=native' if the user does not provide own flags
5ec3142added '-g' as standard compile flag; '-O0' is standard in case of debugging now
8ca0b48much simpler by using testcflags.sh
ecb7606removed redundant flags -O2, -O0, -g
5bdf4e4remove flag -Wall by default
4543249adding comment about assuming compilers from same suite
bc6f7c1testcflags.sh has moved to build/bin

comment:57 Changed 3 months ago by gh-kliem

What I'm getting from the failures above is the following:

Don't use -march=native as a default for gfortran and and python.

Require gcc at least 5.1 in addition to the checks here (this was my first approach, I don't find the bug anymore that caused me to think we need at least version 5.1, but certainly 4.9.2 isn't working for us).

comment:58 Changed 3 months ago by mkoeppe

This change is broken (empty then clause)

diff --git a/build/pkgs/pari/spkg-check.in b/build/pkgs/pari/spkg-check.in
index ae3aecb..a353b0e 100644
--- a/build/pkgs/pari/spkg-check.in
+++ b/build/pkgs/pari/spkg-check.in
@@ -3,10 +3,8 @@
 ###########################################
 
 if [ "x$SAGE_DEBUG" = xyes ] ; then
-    CFLAGS="$CFLAGS -O0 -g" # Disable optimisation, add debug symbols. Good
-                            # for debugging or working around compiler bugs.
 else
-    CFLAGS="-O3 -g $CFLAGS" # Default optimisation, with debug symbols.
+    CFLAGS="-O3 $CFLAGS" # Default optimisation, with debug symbols.
                             # Prepend to not override user's setting.
 fi
 

Reported by vbraun at #29589

comment:59 in reply to: ↑ 52 Changed 3 months ago by gh-kliem

Replying to mkoeppe:

Also segfault on linuxmint-19.3-minimal (https://github.com/mkoeppe/sage/runs/617198994)

Definitely not ready for 9.1.

This appears to be #28800. I'm currently using the original settings for

  • pari (to fix this segemtation fault),
  • fflas_ffpack,
  • gfortran.

I'll push something, once I have more luck.

comment:60 Changed 3 months ago by gh-kliem

  • Branch changed from public/27122-reb3 to public/27122-reb4
  • Commit changed from bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0 to a04ca1e95e6af5f55e59be2832710eab5cd52c6b

New commits:

03b1734Merge branch 'public/27122-reb3' of git://trac.sagemath.org/sage into public/27122-reb4
a04ca1eattempt to fix march=native on several platforms

comment:61 Changed 3 months ago by git

  • Commit changed from a04ca1e95e6af5f55e59be2832710eab5cd52c6b to 072e7e05ab233095c95bdbfb54b5bd984cc378d6

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

072e7e0typo

comment:62 Changed 3 months ago by gh-kliem

  • Status changed from new to needs_info

It seems that my approaches for disabling -march=native for certain packages don't work.

For ubuntu-trusty it tried to compile gfortran with -march=native, but I tried to disable that.

https://github.com/kliem/sage-test-27122/runs/622914460?check_suite_focus=true

For linuxmint-19, python 2 I'm still getting the segmentation fault when building the documentation. So I'm suspecting that pari was build with -march=native against my wishes.

https://github.com/kliem/sage-test-27122/runs/622915243?check_suite_focus=true

I can keep on trying, but maybe someone that understands those scripts better than me finds a mistake. I would appreciate that.

comment:63 Changed 2 months ago by git

  • Commit changed from 072e7e05ab233095c95bdbfb54b5bd984cc378d6 to 31db2e54c451c44b18234cfe839d5c6799e0922a

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

1ac06d5further tries
1069f65original cflags for cypari
026b89freally check for gcc >= 5.1
8a578ddcflags for pari_jupyter
31db2e5makefile syntax

comment:64 Changed 2 months ago by gh-kliem

  • Cc embray added

Turns out my makefile syntax was completely wrong. But I think I got it figured out now and it should correctly determine the gcc version (and check correctly SAGE_DEBUG and SAGE_FAT_BINARY).

@embray: As you wrote a workaround that fixes segmentation faults when building the documentation in #28356, maybe you have an educated guess, which package might be causing the following problem. Currently I disable -march=native for the following:

  • pari
  • cypari
  • pari_jupyter
  • fflas_ffpack
  • gfortran
  • gcc (I'm not sure if gcc/build_gcc will do that, it seemed to help the gfortran issue however)
  • linbox
[combinat ] building [inventory]: targets for 368 source files that are out of date
[combinat ] updating environment: 368 added, 0 changed, 0 removed
Error building the documentation.
Traceback (most recent call last):
  File "/sage/local/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/sage/local/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/sage/local/lib/python3.7/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
    main()
  File "/sage/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 1720, in main
    builder()
  File "/sage/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 327, in _wrapper
    getattr(get_builder(document), 'inventory')(*args, **kwds)
  File "/sage/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 552, in _wrapper
    self._build_everything_except_bibliography(lang, format, *args, **kwds)
  File "/sage/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 538, in _build_everything_except_bibliography
    build_many(build_ref_doc, non_references)
  File "/sage/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 280, in build_many
    _build_many(target, args, processes=NUM_THREADS)
  File "/sage/local/lib/python3.7/site-packages/sage_setup/docbuild/utils.py", line 283, in build_many
    raise worker_exc.original_exception
Exception: ('Non-exception during docbuild: Segmentation fault', SignalError('Segmentation fault'))

@all: Is there a way to obtain a meaningful backtrace? Antonio Rojas stated that he downgraded sage to obtain that. But that is not really an option here. https://groups.google.com/d/msg/sage-packaging/VU4h8IWGFLA/rU-8NCPjBgAJ

comment:65 follow-up: Changed 2 months ago by mkoeppe

I think determining flags should really be done in configure. Also let's add configure options for enabling SAGE_FAT_BINARY

comment:66 Changed 2 months ago by mkoeppe

likewise for SAGE_DEBUG

comment:67 Changed 2 months ago by gh-kliem

Yes, I also realized that.

When I was looking for which flags the packages where using, I always looked into the configure file, because that's the place where it belongs to :-) It took me a while to realize that I'm doing it wrong then.

comment:68 Changed 7 weeks ago by saraedum

Sorry, haven't read through all the discussion, so apologies if this already came up. The gcc manpage recommends -Og -g to "Optimize debugging experience" over -O0 and the resulting code runs much faster.

comment:69 Changed 4 weeks ago by gh-kliem

  • Branch changed from public/27122-reb4 to u/gh-kliem/march_native_in_configure
  • Commit changed from 31db2e54c451c44b18234cfe839d5c6799e0922a to f63aa174447b9b214fae82eb26727ddbea25f5b5

New try. Do it in configure.ac now, this is where it belongs I think. I will most likely have all the old problems again.


New commits:

f63aa17add march=native in configure.ac

comment:70 in reply to: ↑ 65 ; follow-up: Changed 4 weeks ago by gh-kliem

Replying to mkoeppe:

I think determining flags should really be done in configure. Also let's add configure options for enabling SAGE_FAT_BINARY

What kind of options should I add for this?

Something like --disable-sse --disable-sse2 --disable-sse3 --disable-ssse3 --disable-sse41 --disable-sse42 --disable-fma --disable-fma4 --disable-avx --disable-avx2? However, I'm not sure that this will be used anywhere.

comment:71 Changed 4 weeks ago by git

  • Commit changed from f63aa174447b9b214fae82eb26727ddbea25f5b5 to 6e407f7b89dc1017a036edd466e81bc37d0ae91c

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

233af27remove reduntant flag setting in packages
6e407f7Merge branch 'u/gh-kliem/march_native_in_configure' of git://trac.sagemath.org/sage into u/gh-kliem/march_native_in_configure

comment:72 Changed 4 weeks ago by dimpase

we configure compilers mostly in build/pkgs/{gcc,gfortran}/spkg-configure.m4 - could you perhaps move (most of) your changes to configure.ac there?

comment:73 follow-up: Changed 4 weeks ago by gh-kliem

I don't think this is where it belongs. I'm setting flags for building any package.

This is usually done in configure.ac and is so already. If you don't set CFLAGS then AC_PROG_CC() will set CFLAGS to -g -02 (or different order).

comment:74 in reply to: ↑ 73 Changed 4 weeks ago by dimpase

Replying to gh-kliem:

I don't think this is where it belongs. I'm setting flags for building any package.

sure, you are setting compilers' flags, and this is done in the files I pointed to. E.g. in build/pkgs/gcc/spkg-configure.m4 we have

    # Modify CXX to include an option that enables C++11 support if necessary
    AX_CXX_COMPILE_STDCXX_11([], optional)

which adds if needed something to CXXFLAGS, etc.

This is usually done in configure.ac and is so already. If you don't set CFLAGS then AC_PROG_CC() will set CFLAGS to -g -02 (or different order).

AC_PROG_CC only is called early in configure.ac to ensure general sanity. As you might know, build/pkgs/*/spkg-configure.m4 are collected by ./boostrap and used to generate ./configure from them and configure.ac. We have moved a lot of compiler-specific stuff into build/pkgs/*/spkg-configure.m4 when the latter were instroduced, and for a good reason.

comment:75 in reply to: ↑ 70 ; follow-up: Changed 4 weeks ago by mkoeppe

Replying to gh-kliem:

Replying to mkoeppe:

I think determining flags should really be done in configure. Also let's add configure options for enabling SAGE_FAT_BINARY

What kind of options should I add for this?

Something like --disable-sse --disable-sse2 --disable-sse3 --disable-ssse3 --disable-sse41 --disable-sse42 --disable-fma --disable-fma4 --disable-avx --disable-avx2? However, I'm not sure that this will be used anywhere.

What I meant by this was for the top-level configure to have an option --enable-fat-binary (or perhaps a better name for that) so that this is not controlled by an environment variable.

comment:76 Changed 4 weeks ago by git

  • Commit changed from 6e407f7b89dc1017a036edd466e81bc37d0ae91c to 6ba54119a79ce724cccdc3d48c0661d1d7320af8

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

6ba5411enable-fat-binary

comment:77 in reply to: ↑ 75 Changed 4 weeks ago by gh-kliem

Replying to mkoeppe:

Replying to gh-kliem:

Replying to mkoeppe:

I think determining flags should really be done in configure. Also let's add configure options for enabling SAGE_FAT_BINARY

What kind of options should I add for this?

Something like --disable-sse --disable-sse2 --disable-sse3 --disable-ssse3 --disable-sse41 --disable-sse42 --disable-fma --disable-fma4 --disable-avx --disable-avx2? However, I'm not sure that this will be used anywhere.

What I meant by this was for the top-level configure to have an option --enable-fat-binary (or perhaps a better name for that) so that this is not controlled by an environment variable.

Yes this should make things easier.

comment:78 Changed 4 weeks ago by git

  • Commit changed from 6ba54119a79ce724cccdc3d48c0661d1d7320af8 to a6b93f261b30adfdb37d8867dbb05e0713e32d8d

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

a6b93f2typo for iml

comment:79 Changed 4 weeks ago by gh-kliem

  • Status changed from needs_info to needs_review

It seems that with the new approach things have improved. Or maybe some work has been done somewhere else.

  • tox.ini:

looks the same to me (new failure to push docker image for debian-jessie):

beta1: https://github.com/sagemath/sage/actions/runs/134966983

#27122: https://github.com/kliem/sage/actions/runs/136808684

  • optional:

no new failure (although tests aren't all the way done yet)

beta1: https://github.com/sagemath/sage/actions/runs/134966981

#27122: https://github.com/kliem/sage/actions/runs/136808682

  • gcc_spkg:

some tests take half an hour longer, but I don't know if that has any say

beta1: https://github.com/sagemath/sage/actions/runs/134966986

#27122: https://github.com/kliem/sage/actions/runs/136808677

  • cygwin-standard:

looks the same to me:

beta1: https://github.com/sagemath/sage/actions/runs/134966985

#27122: https://github.com/kliem/sage/actions/runs/136808676

  • cygwin-minimal:

looks the same to me:

beta1: https://github.com/sagemath/sage/actions/runs/134966984

https://github.com/kliem/sage/actions/runs/136808673

comment:80 Changed 4 weeks ago by gh-kliem

  • Status changed from needs_review to needs_info

Never mind. The environment variables CFLAGS never made it from configure.ac to anywhere else.

I'm trying to figure out, who to do this. Any help welcome.

comment:81 Changed 4 weeks ago by git

  • Commit changed from a6b93f261b30adfdb37d8867dbb05e0713e32d8d to c225c7ed5260b64dce8748c9aa92502874f2cd39

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

39652accopy/paste typo
c225c7eexport configured compilation flags for build

comment:82 Changed 4 weeks ago by gh-kliem

  • Status changed from needs_info to needs_work

I think I found the correct place.

Now it's probably going to fail on certain machines again.

comment:83 Changed 4 weeks ago by dimpase

  • Cc fbissey added

once again, settings of gcc flags are not to be done in configure.ac - as I explained, we already have all of them we currently set, set in build/pkgs/{gcc,gfortran}/spkg-configure.m4

Please move them accordingly, or if you have problem doing that, I can help, but doing compiler settings in two different places is verboden.

comment:84 Changed 3 weeks ago by gh-kliem

Well, configure.ac runs AC_PROG_CC(), which sets CFLAGS to -g -O2 if not set, so we must grep the user CFLAGS in configure.ac (it was already pointed out above that user input CFLAGS should not be overridden unless necessary).

If I get this right, configure.ac just barely checks if there is anything that can be used to at least build gcc. In gcc we futher analyse what the user has and figure out, whether this suffices.

I agree that it makes sense to figure out the flags there as we are doing all those case distinctions already (what compiler are you etc, can you compile a basic file etc.). I'll take care of it soon, but not today (as for testing it shouldn't really make a difference, there are many open problems with this).

comment:85 follow-up: Changed 3 weeks ago by dimpase

I think AC_PROG_CC etc can be moved to gcc/gfortran's spkg-configure.m4

comment:86 in reply to: ↑ 85 Changed 3 weeks ago by dimpase

Replying to dimpase:

I think AC_PROG_CC etc can be moved to gcc/gfortran's spkg-configure.m4

I did a very quick check, applying

  • configure.ac

    a b AX_PROG_PERL_VERSION([5.8.0],[],[ 
    308308# Check C/C++/Fortran compilers
    309309###############################################################################
    310310
     311SAGE_SPKG_COLLECT()
    311312SAGE_CHECK_CONDA_COMPILERS
    312313
    313314AC_PROG_CC()
    AS_IF([test "x$enable_download_from_upstream_url" = "xyes"], [ 
    412413])
    413414AC_SUBST([SAGE_SPKG_OPTIONS])
    414415
    415 SAGE_SPKG_COLLECT()
    416416
    417417# We need sage-env-config to exist before running this.
    418418# TODO: fix this in Trac #21524

and everything still seemed to be working. This means that we can move all these AC_PROG_*() (perhaps using AC_REQUIRE()) to respective build/pkgs/*/*.m4 files.

If you could try this during the refactoring you're planning to do, it'd be great.

comment:87 Changed 3 weeks ago by git

  • Commit changed from c225c7ed5260b64dce8748c9aa92502874f2cd39 to 4596c4bcb5e9132c4e627edd8081ed139b22e613

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

b8b5409O3 non native
2e11178non-native for eclib
3850dcbexport SAGE_FAT_BINARY of configure
be72e42r non-native
4596c4bpari without native

comment:88 Changed 3 weeks ago by gh-kliem

  • Status changed from needs_work to needs_info

And back to the old problems again.

https://github.com/kliem/sage/actions/runs/140694723

There are sementation faults when building the documentation e.g. in ubuntu bionic.

However, I disabled march=native for pari and all packages depending on pari and that still didn't seem to fix the issue (according to the logs this seemed to work).

Something isn't thread safe from above probably as in #28800. I have no clue, about that and I'm left with disabling march=native for random packages and hoping that I have luck.

Or does anyone know how to get a meaningful log of the documentation build?

comment:89 Changed 3 weeks ago by gh-kliem

One thing one could do, is to not build the documentation and hope that the problem shows up during the doctests.

comment:90 follow-up: Changed 3 weeks ago by fbissey

permutahedron. Looks like stuff linked to cddlib, it says so in the code. I don't remember cddlib to be so sensitive to optimisation. Heck, I use -march=native in my sage-on-gentoo builds and I am fine. But pari is definitely sensitive to optimisation. When I was working on enabling clang, I also tried the Intel compiler. pari needed to be compiled at -O0 with it. Any optimisation with icc would break it. That was the only package that I remember with that issue.

comment:91 Changed 3 weeks ago by gh-kliem

Thanks. I'm going through the packages now. It might have been lcalc after all though. Turns out it also uses CFLAGS, but we never minded that in spkg-install. And as it uses the pari headers, it might recompile something and then lead to the problem.

comment:92 Changed 3 weeks ago by gh-kliem

And it wasn't cddlib, at least not by itself.

comment:93 Changed 3 weeks ago by gh-kliem

  • Work issues set to move to gcc/spkg-configure; fix documentation build

comment:94 Changed 3 weeks ago by gh-kliem

  • Status changed from needs_info to needs_review

I think I found the problem. After disabling gap, it worked.

https://github.com/kliem/sage/actions/runs/141805272

comment:95 Changed 3 weeks ago by git

  • Commit changed from 4596c4bcb5e9132c4e627edd8081ed139b22e613 to 4c52d4d43794f807875739deb27a51d46836764c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4c52d4dgap without native

comment:96 Changed 3 weeks ago by gh-kliem

  • Description modified (diff)
  • Status changed from needs_review to needs_work
  • Work issues changed from move to gcc/spkg-configure; fix documentation build to move to gcc/spkg-configure; enable-debug; documententation for developers

I meant to change from "needs info" to "needs work" earlier.

comment:97 Changed 3 weeks ago by git

  • Commit changed from 4c52d4d43794f807875739deb27a51d46836764c to d12ed68a2a7a6fb9161cb11f066ca94c4b6c228e

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

9f958cfupdate documentation
441002c--enable-debug as configuration option
74b82b6move to buil/spkg/{gcc,gfortran}/spkg-configure.ac
d12ed68disable native for gcc build

comment:98 Changed 3 weeks ago by gh-kliem

  • Status changed from needs_work to needs_review
  • Work issues move to gcc/spkg-configure; enable-debug; documententation for developers deleted
Last edited 3 weeks ago by gh-kliem (previous) (diff)

comment:99 Changed 3 weeks ago by gh-kliem

Somehow it seems the second last commit 74b82b6506cd71d87f616389fe2fa142dd16f4cf has messed up the scipy build. Or something else is going on. I'm getting:

 [sagelib-9.2.beta1] installing. Log file: /sage/logs/pkgs/sagelib-9.2.beta1.log
  [sagelib-9.2.beta1] successfully installed.
  [scipy-1.2.3] error installing, exit status 1. End of log file:
  [scipy-1.2.3]       Running setup.py install for scipy: finished with status 'error'
  [scipy-1.2.3]   Cleaning up...
  [scipy-1.2.3]     Removing source in /tmp/pip-req-build-18apw96c
  [scipy-1.2.3]   Removed build tracker '/tmp/pip-req-tracker-x3hgraz6'
  [scipy-1.2.3]   Command "/sage/local/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-req-build-18apw96c/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" --no-user-cfg install --record /tmp/pip-record-v0usfnl4/install-record.txt --single-version-externally-managed --root /sage/local/var/tmp/sage/build/scipy-1.2.3/inst --compile --install-headers /sage/local/include/site/python3.7/scipy" failed with error code 1 in /tmp/pip-req-build-18apw96c/
  [scipy-1.2.3]   Exception information:
  [scipy-1.2.3]   Traceback (most recent call last):
  [scipy-1.2.3]     File "/sage/local/lib/python3.7/site-packages/pip/_internal/cli/base_command.py", line 143, in main
  [scipy-1.2.3]       status = self.run(options, args)
  [scipy-1.2.3]     File "/sage/local/lib/python3.7/site-packages/pip/_internal/commands/install.py", line 366, in run
  [scipy-1.2.3]       use_user_site=options.use_user_site,
  [scipy-1.2.3]     File "/sage/local/lib/python3.7/site-packages/pip/_internal/req/__init__.py", line 49, in install_given_reqs
  [scipy-1.2.3]       **kwargs
  [scipy-1.2.3]     File "/sage/local/lib/python3.7/site-packages/pip/_internal/req/req_install.py", line 791, in install
  [scipy-1.2.3]       spinner=spinner,
  [scipy-1.2.3]     File "/sage/local/lib/python3.7/site-packages/pip/_internal/utils/misc.py", line 705, in call_subprocess
  [scipy-1.2.3]       % (command_desc, proc.returncode, cwd))
  [scipy-1.2.3]   pip._internal.exceptions.InstallationError: Command "/sage/local/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-req-build-18apw96c/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" --no-user-cfg install --record /tmp/pip-record-v0usfnl4/install-record.txt --single-version-externally-managed --root /sage/local/var/tmp/sage/build/scipy-1.2.3/inst --compile --install-headers /sage/local/include/site/python3.7/scipy" failed with error code 1 in /tmp/pip-req-build-18apw96c/
  [scipy-1.2.3]   Error: installing with pip3 failed

on many platforms. Seems like I messed up when moving stuff around from configure.ac to {gcc,gfortran}/spkg-configure.ac.

comment:100 Changed 3 weeks ago by git

  • Commit changed from d12ed68a2a7a6fb9161cb11f066ca94c4b6c228e to 16a6989274d87c44815f05dab041c5ea825db759

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

16a6989partly undo reordering

comment:102 Changed 3 weeks ago by dimpase

OK, this still looks acceptable, w.r.t. what is done in configure.ac, and try further refactoring on another ticket.

comment:103 Changed 3 weeks ago by mkoeppe

I took a brief look at the source changes, and it looks clean to me. I haven't looked at the tests.

comment:104 Changed 3 weeks ago by gh-kliem

Here is a overview, over the test results (all in comparison to the 9.2.beta1 tests and only mentioning differences):

  • tox.ini:
    • ubuntu-trusty-standard sage -t src/sage/interfaces/maxima.py # Timed out after
      sage: maxima._batch('10003;') ## line 864 ##
      'kill(sage121)$batchload("/root/.sage/temp/9b341214822c/14051/interface/tmp14054-1371013559");1+952962022;\r\n\r\n(%o771) "/root/.sage/temp/9b341214822c/14051/interface/tmp14054-1371013559"\r\n(%o772) '
      sage: maxima._batch('10003;',batchload=False) ## line 866 ##
      
    • debian-stretch-standard: didn't finish in 6 hours
    • linuxmint-19.3-minimal: didn't finish in 6 hours
    • fedora-26, standard: sage -t src/sage/symbolic/expression.pyx # 1 doctest failed
    • fedora-27, minimal: failed to build mpir
    • fedora-28, standard: sage -t src/sage/interfaces/maxima.py # Timed out
    • fedora-31, minimal: sage -t src/sage/symbolic/integration/integral.py # 1 doctest failed7
    • fedora-32, standard didn't finish in time and sage -t src/sage/interfaces/maxima.py # Timed out
    • local-macos (homebrew-macos-python3_xcode, minimal): new failures
      sage -t src/sage/manifolds/differentiable/euclidean.py  # Timed out
      sage -t src/sage/manifolds/differentiable/metric.py  # Timed out
      sage -t src/sage/parallel/map_reduce.py  # 1 doctest failed
      sage -t src/sage/plot/plot3d/platonic.py  # Timed out
      sage -t src/sage/plot/plot3d/shapes.pyx  # Timed out
      
    • local-macos (homebrew-macos-python3_xcode, standard): new failures
      sage -t src/doc/en/thematic_tutorials/vector_calculus/vector_calc_advanced.rst  # Timed out
      sage -t src/doc/en/thematic_tutorials/vector_calculus/vector_calc_cartesian.rst  # Timed out
      sage -t src/sage/geometry/polyhedron/base.py  # Timed out
      sage -t src/sage/manifolds/differentiable/affine_connection.py  # Timed out
      sage -t src/sage/manifolds/differentiable/degenerate_submanifold.py  # Timed out
      sage -t src/sage/manifolds/differentiable/euclidean.py  # Timed out
      sage -t src/sage/manifolds/differentiable/metric.py  # Timed out
      sage -t src/sage/manifolds/differentiable/multivectorfield.py  # Timed out
      sage -t src/sage/manifolds/differentiable/pseudo_riemannian_submanifold.py  # Timed out
      sage -t src/sage/numerical/backends/glpk_backend.pyx  # 1 doctest failed
      sage -t src/sage/parallel/map_reduce.py  # 1 doctest failed
      sage -t src/sage/plot/plot3d/platonic.py  # Timed out
      sage -t src/sage/plot/plot3d/shapes.pyx  # Timed out
      sage -t src/sage/rings/function_field/function_field.py  # Timed out
      sage -t src/sage/schemes/elliptic_curves/isogeny_class.py  # Timed out
      
    • local-macos (homebrew-macos-python3_xcode-nokegonly, minimal)
      sage -t src/sage/plot/plot3d/shapes.pyx  # Timed out
      sage -t src/sage/rings/function_field/function_field.py  # Timed out
      
    • local-macos (homebrew-macos-python3_xcode-nokegonly, standard)
      sage -t src/sage/plot/plot3d/shapes.pyx  # Timed out
      sage -t src/sage/rings/function_field/function_field.py  # Timed out
      
    • local-macos (homebrew-macos-python3_pythonorg, minimal)
      sage -t src/sage/manifolds/differentiable/euclidean.py  # Timed out
      sage -t src/sage/manifolds/differentiable/metric.py  # Timed out
      sage -t src/sage/plot/plot3d/shapes.pyx  # Timed out
      sage -t src/sage/rings/function_field/function_field.py  # Timed out
      
    • local-macos (homebrew-macos-python3_pythonorg, standard)
      sage -t src/sage/manifolds/differentiable/euclidean.py  # Timed out
      sage -t src/sage/manifolds/differentiable/metric.py  # Timed out
      sage -t src/sage/parallel/map_reduce.py  # 1 doctest failed
      sage -t src/sage/plot/plot3d/platonic.py  # Timed out
      sage -t src/sage/plot/plot3d/shapes.pyx  # Timed out
      sage -t src/sage/schemes/elliptic_curves/isogeny_class.py  # Timed out
      sage -t src/sage/symbolic/relation.py  # Timed out
      
    • local-macos (conda-forge-macos): many failures, looks like the new ones are the same as other mac

I'll leave it at that for now.

All those new mac failures don't look good.

comment:105 in reply to: ↑ 90 Changed 3 weeks ago by gh-kliem

Replying to fbissey:

permutahedron. Looks like stuff linked to cddlib, it says so in the code. I don't remember cddlib to be so sensitive to optimisation. Heck, I use -march=native in my sage-on-gentoo builds and I am fine. But pari is definitely sensitive to optimisation. When I was working on enabling clang, I also tried the Intel compiler. pari needed to be compiled at -O0 with it. Any optimisation with icc would break it. That was the only package that I remember with that issue.

It really looks like pari shouldn't be optimzed. At least not when using clang. https://github.com/kliem/sage/runs/787575732?check_suite_focus=true

This old run, didn't have the new doctest failures.

So try disabling -march=native for pari and maxima`?

comment:106 Changed 3 weeks ago by git

  • Commit changed from 16a6989274d87c44815f05dab041c5ea825db759 to 863862679f6a9cba3476b515dbbfe22ed31a6571

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

3a43cd1pari without native
8638626lcalc for real

comment:107 follow-up: Changed 3 weeks ago by gh-kliem

maxima doesn't compile with -march=native, so I guess this has nothing to do with it. Hopefully the new run without pari will resolve the other problems:

comment:108 in reply to: ↑ 107 Changed 3 weeks ago by dimpase

Replying to gh-kliem:

maxima doesn't compile with -march=native, so I guess this has nothing to do with it.

As far as I understand, Maxima build process does not involve anything but a Lisp compiler, ECL in the case of Sage. And ECL compiles directly into object files, as far as I know.

So this is probably a red herring.

comment:109 Changed 2 weeks ago by gh-kliem

The last commits don't make a difference as far as I can tell. But the new mac failures seem to be real random. They are also present here (which is a completely different ticket):

I personally guess the new errors on mac are either random or caused by something else (some package updated maybe). That would also explain why on homebrew-macos-python3_xcode there are so many new failures for standard, but not for minimal. (As this ticket here is supposed to influence minimal much more than standard.)

Something is going on either way, as even on 9.2.beta1 we have many time outs in plot and manifolds. So if I get an extra error in sage -t src/sage/plot/plot3d/shapes.pyx, I don't even know if this is really bad or just random.

Basically, I see the following options now:

  • revert the last two commits or not (independent of the other options):

I don't see an advantage in disabling -march=native for pari, but maybe that is really something we should do.

  • decide that the macos failures are either random or acceptable
  • disable -march=native for clang (but it is not said that things would improve for mac then)
  • someone has an educated guess about what I could do about the mac failures

It is possible however that some of the new mac failures are caused by the fact that the default CFLAGS is now -g -O2 instead of just empty.

For an updated comparison, I started a mac only test on the current develop again: https://github.com/kliem/sage/actions/runs/148471033

comment:110 Changed 2 weeks ago by git

  • Commit changed from 863862679f6a9cba3476b515dbbfe22ed31a6571 to 28ac47550eebe0ad6cd3094b432bed29085838ff

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

5d58cbeRevert "lcalc for real"
28ac475Revert "pari without native"

comment:111 Changed 2 weeks ago by gh-kliem

I can confirm that the mac os errors also happen on a clean build from 9.2.beta1, so I think it has nothing to do with that ticket and is just somewhat random behaviour:

local-macos (homebrew-macos-python3_pythonorg, minimal) https://github.com/kliem/sage/runs/810515848?check_suite_focus=true

had the following failures:

sage -t src/sage/dynamics/arithmetic_dynamics/projective_ds.py  # Timed out
sage -t src/sage/interfaces/gap.py  # 1 doctest failed
sage -t src/sage/manifolds/differentiable/euclidean.py  # Timed out
sage -t src/sage/manifolds/differentiable/metric.py  # Timed out
sage -t src/sage/manifolds/differentiable/tensorfield.py  # Timed out
sage -t src/sage/parallel/map_reduce.py  # 1 doctest failed
sage -t src/sage/plot/plot.py  # Timed out
sage -t src/sage/plot/plot3d/base.pyx  # Timed out
sage -t src/sage/plot/plot3d/implicit_plot3d.py  # Timed out
sage -t src/sage/plot/plot3d/parametric_plot3d.py  # Timed out
sage -t src/sage/plot/plot3d/plot3d.py  # Timed out
sage -t src/sage/plot/plot3d/shapes.pyx  # Timed out
sage -t src/sage/plot/plot3d/shapes2.py  # Timed out
sage -t src/sage/rings/function_field/function_field.py  # Timed out
sage -t src/sage/schemes/elliptic_curves/ell_number_field.py  # Timed out

I would propose leaving the ticket as it is. Testwise I think it looks fine.

comment:112 Changed 2 weeks ago by mkoeppe

Could you comment on whether this implements autotools semantics of "precious variables" (see #29507) for CFLAGS, FCFLAGS etc.?

comment:113 Changed 2 weeks ago by gh-kliem

This definitely doesn't implement autotools semantics of "precious variables", but I think it is an improvement towards that.

Until now, every build of sage would pick up CFLAGS, CXXFLAGS, FCFLAGS and F77FLAGS again from the environment. This is strange especially now that we have seperated make from configure.

With this ticket the flags will be picked up on configure (if set) and will overwritten for make according to their set up (this is what sage-build-env-config.in does). They are also mostly respected now. Until now, if you set CFLAGS to -O0, every package spkg-install.in would just overwrite that (or add -g -O2, which in practice overwrites it I think). Now many (but not all) packages will just respect that.

I think it needs very little change to implement autotools semantics of "precious variables" CFLAGS etc. from here, but I'm not sure exactly how to do this.

comment:114 Changed 2 weeks ago by git

  • Commit changed from 28ac47550eebe0ad6cd3094b432bed29085838ff to 234338ea6283549195c4b22fc84cb621fcb75c48

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

234338eexport SAGE_DEBUG, if this was enabled during configure

comment:115 Changed 2 weeks ago by gh-kliem

Regarding the last commit, we should really pick up SAGE_DEBUG from configuration time and not from build time. It will influence the build of some packages.

comment:116 Changed 8 days ago by mkoeppe

Can users still use the SAGE_DEBUG environment variable at all to configure anythin, or is it overridden by the enable value?

comment:117 Changed 8 days ago by gh-kliem

+AC_ARG_ENABLE([debug],
+              [AS_HELP_STRING([--enable-debug={no|symbols|yes}],
+                              [controls debugging support: "no" debugging; debugging "symbols" (default); build debug version ("yes")])],
+              [AC_SUBST(SAGE_DEBUG, $enableval)],
+              [])
+

Yes you can still set SAGE_DEBUG. However, if you configure with the option --enable-debug=..., it will override this variable.

Note also that this must now be done at configuration time.

I just checked. The following all work:

./configure --enable-debug=yes
./configure SAGE_DEBUG=yes
export SAGE_DEBUG=yes; ./configure

comment:118 follow-up: Changed 6 days ago by mkoeppe

I think it would be good if at least make SAGE_DEBUG=yes some-package would still be supported.

comment:119 in reply to: ↑ 118 Changed 6 days ago by gh-kliem

Replying to mkoeppe:

I think it would be good if at least make SAGE_DEBUG=yes some-package would still be supported.

The problem is that what SAGE_DEBUG mostly does is to set CFLAGS accordingly. If we configure this stuff in configure then it feels like doing everything twice. Unless we ignore SAGE_DEBUG mostly in configure and wait for this until sage-build-env-config.in. There we could do

export SAGE_DEBUG?="@SAGE_DEBUG@"

and then build together all the flags according to SAGE_DEBUG.

comment:120 Changed 6 days ago by mkoeppe

This sounds like a good solution.

I think it's important to support per-package SAGE_DEBUG, just like we support per-package SAGE_CHECK.

comment:121 Changed 6 days ago by git

  • Commit changed from 234338ea6283549195c4b22fc84cb621fcb75c48 to e9fb1a37af6ac08c29dff5a9cb062eccf60f761a

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

df53cf3--enable-debug as configuration option
6fd7ba7move to buil/spkg/{gcc,gfortran}/spkg-configure.ac
03b002adisable native for gcc build
0660ab0partly undo reordering
5eba95apari without native
1102ac1lcalc for real
d412f06Revert "lcalc for real"
453c97bRevert "pari without native"
f23e68cexport SAGE_DEBUG, if this was enabled during configure
e9fb1a3allow debuggin of individual packages

comment:122 Changed 6 days ago by gh-kliem

Ok. I went back into build/make/Makefile.in. I think it works now.

If you run make build SAGE_DEBUG=yes without configuration, it will still assemble according to SAGE_DEBUG. You can even choose different flags with make CFLAGS=-O2 some-package. That will also work.

However export SAGE_DEBUG=yes; make build will not work, unless for some reason this will induce configuration.

comment:123 Changed 6 days ago by gh-kliem

  • Description modified (diff)
Note: See TracTickets for help on using tickets.