#27122 closed enhancement (fixed)
Compile with march=native
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.3 
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:  7bc1f28 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #30375  Stopgaps: 
Description (last modified by )
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 add march=native
to the compiler flags, if not building fat binaries and the compiler and the package accepts it.
During the build of sage there are now the following 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 withmarch=native
if possible)export CFLAGS=$CFLAGS_NON_NATIVE
(flags as above withoutmarch=native
)export CFLAGS=$CFLAGS_O3
(O3
instead ofO2
, but only if not in debug mode, addmarch=native
if possible)export CFLAGS=$CFLAGS_O3_NON_NATIVE
(O3
instead ofO2
, nomarch=native
)export CFLAGS=$ORIGINAL_CFLAGS
(use$CFLAGS
as set beforeconfigure
)
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 somepackage
or
make SAGE_DEBUG="yes" somepackage
Change History (146)
comment:1 Changed 4 years ago by
 Description modified (diff)
 Keywords Polyhedra FindPoly removed
 Summary changed from Adding march=native to build/cythonized/setup.py to Compile with march=native
comment:2 Changed 4 years ago by
 Description modified (diff)
comment:3 Changed 3 years ago by
 Milestone changed from sage8.7 to sage8.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 3 years ago by
 Milestone sage8.8 deleted
As the Sage8.8 release milestone is pending, we should delete the sage8.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 (sage8.9).
comment:5 Changed 3 years ago by
 Priority changed from minor to major
comment:6 Changed 3 years ago by
 Branch set to public/27122
 Commit set to cdc86563cfe52a78e9e50b1ce374750434e1dc1e
New commits:
cdc8656  compiling with `march=native` for GCC from 5.1 and clang from 6.0 if SAGE_FAT_BINARY is not set

comment:7 Changed 3 years ago by
 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 3 years ago by
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 3 years ago by
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.
[sagelib9.0.beta2] [469/499] gcc fnostrictaliasing g O2 DNDEBUG g fwrapv O3 Wall Wnounused fPIC I./sage/cpython I/srv/public/kliem/sage/local/lib/python2.7/sitepackages/cypari2 I./sage/libs/ntl I/srv/public/kliem/sage/local/lib/python2.7/sitepackages/ 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/sitepackages/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.linuxx86_642.7/build/cythonized/sage/stats/distributions/discrete_gaussian_integer.o fnostrictaliasing DCYTHON_CLINE_IN_TRACEBACK=1 march =native D_XOPEN_SOURCE=600 std=c99 [sagelib9.0.beta2] build/cythonized/sage/stats/hmm/hmm.c: In function ‘__pyx_pw_4sage_5stats_3hmm_3hmm_25DiscreteHiddenMarkovModel_17_forward’:
comment:10 Changed 3 years ago by
 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.06amd64
 Architecture: x8664
 CPU Model name: Intel(R) Core(TM) i77700 CPU @ 3.60GHz
comment:11 Changed 3 years ago by
 Description modified (diff)
comment:12 Changed 3 years ago by
 Cc selia added
comment:13 Changed 3 years ago by
Works on
 Operating System: Ubuntu 18.04.3 LTS
 Kernel: Linux 4.15.065generic
 Architecture: x8664
 Model name: Intel(R) Core(TM) i57200U CPU @ 2.50GHz
comment:14 Changed 3 years ago by
Success on my notebook running a Python 3based 9.0.beta2 on top of Debian testing :
 Operating System: Debian GNU/Linux bullseye/sid
 Kernel: Linux 5.2.03amd64
 Architecture: x8664
 Nom de modèle : Intel(R) Core(TM) i78550U CPU @ 1.80GHz
HTH,
comment:15 Changed 3 years ago by
works on Intel(R) Core(TM) i52500 CPU @ 3.30GHz (in 32bit mode), Ubunty 14.04 LTS
comment:16 Changed 3 years ago by
I don't think we should be reinventing the thirtyyearold 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 3 years ago by
 Commit changed from cdc86563cfe52a78e9e50b1ce374750434e1dc1e to 9aabee1ecf8507cf68a6122ca3febfa0416af528
Branch pushed to git repo; I updated commit sha1. New commits:
9aabee1  respect CFLAGS

comment:18 followup: ↓ 19 Changed 3 years ago by
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 ; followup: ↓ 20 Changed 3 years ago by
Replying to ghkliem:
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 3 years ago by
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 ghkliem:
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 followup: ↓ 23 Changed 3 years ago by
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 safebutefficient 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 spkginstall and strip out any "O2" and "march=native" additions.
comment:22 Changed 3 years ago by
 Branch changed from public/27122 to public/27122new
 Commit changed from 9aabee1ecf8507cf68a6122ca3febfa0416af528 to 60bddf468a3c0b0e70ec285ebf450ca45eabf8f3
New commits:
b66e81d  compile with 'O2 march=native' if the user does not provide own flags

e16ba0e  added 'g' as standard compile flag; 'O0' is standard in case of debugging now

11b9175  much simpler by using testcflags.sh

a574572  removed redundant flags O2, O0, g

60bddf4  remove flag Wall by default

comment:23 in reply to: ↑ 21 Changed 3 years ago by
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 safebutefficient 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 spkginstall and strip out any "O2" and "march=native" additions.
comment:24 Changed 3 years ago by
Is the new code acceptable?
comment:25 Changed 3 years ago by
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 3 years ago by
 Branch changed from public/27122new to public/27122reb
 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:
0045dba  compile with 'O2 march=native' if the user does not provide own flags

4241dfe  added 'g' as standard compile flag; 'O0' is standard in case of debugging now

ea3c0df  much simpler by using testcflags.sh

60e3c36  removed redundant flags O2, O0, g

bec54cc  remove flag Wall by default

e420546  adding comment about assuming compilers from same suite

comment:27 Changed 3 years ago by
comment:28 Changed 3 years ago by
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. withoutsystemfoo
, but there are a lot of flags you have to pass. Something like
$ ./configure help  grep oP '\\withsystem[^ =]+'  sort  uniq  sed 's/with/without/' withoutsystemarb withoutsystematlas withoutsystembzip2 ...
can be used to get the full list for your branch.
comment:29 Changed 3 years ago by
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 '\\withsystem[^ =]+'  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 '\\withsystem[^ =]+'  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 '\\withsystem[^ =]+'  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 '\\withsystem[^ =]+'  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.
comment:30 followup: ↓ 31 Changed 3 years ago by
comment:31 in reply to: ↑ 30 Changed 3 years ago by
https://github.com/kliem/sagetest27122/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
 ubuntuxenial, minimal, ubuntulatest
 ubuntuxenial, standard, ubuntulatest
 Problems with gsl and libatomic_ops for
 ubuntubionic, standard, ubuntulatest
 ubuntueoan, standard, ubuntulatest
 fedora26, standard, ubuntulatest
 fedora28, standard, ubuntulatest
 Never ran the following:
 ubuntubionici386, 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 2 years ago by
 Branch changed from public/27122reb to public/27122reb2
 Commit changed from e4205465daa4267c12a3ff49d23471b277bc16d5 to 301bd2c279991115c5702b1837118339360e8e86
New commits:
dc0e744  compile with 'O2 march=native' if the user does not provide own flags

f8ce3b3  added 'g' as standard compile flag; 'O0' is standard in case of debugging now

f645f71  much simpler by using testcflags.sh

69b6d35  removed redundant flags O2, O0, g

81a6ea5  remove flag Wall by default

301bd2c  adding comment about assuming compilers from same suite

comment:33 followup: ↓ 34 Changed 2 years ago by
Can someone please help me with the following issue.
 condaforge and homebrewmacos
`/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/sagetest27122/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.
 ubuntutrusty, 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)
 ubuntutrusty, standard
 sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
Got cancelled at
sage t src/sage/modular/local_comp/liftings.py
 ubuntutrusty, standardpython2
 sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
 sage t src/sage/doctest/test.py https://groups.google.com/d/msg/sagerelease/kU5M1QVuQQY/IEXj4PDAwAJ
Got cancelled at
sage t src/sage/schemes/affine/affine_subscheme.py
 ubuntuxenial, minimal  ubuntubionic, minimal  debianjessie, minimal  debianbuster, minimal  fedora26, minimal  fedora26, standard  centos8, standard  archlinuxlatest, 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
 ubuntuxenial, standard  ubuntuxenial, standardpython2
 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
 ubuntubionic, standard  ubuntubionic, standardpython2  ubuntufocal, standard  ubuntufocal, standardpython2  linuxmint19.3, standard, linuxmint19.3, standardpython2
 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
 ubuntueoan, minimal
Got cancelled at
sage t src/sage/manifolds/differentiable/differentiable_submanifold.py
 ubuntueoan, standard
E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/u/underscore/libjsunderscore_1.8.3~dfsg1_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/libjssphinxdoc_1.6.71ubuntu1_all.deb Unable to connect to azure.archive.ubuntu.com:http: E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/p/pythonpluggy/python3pluggy_0.6.01_all.deb Unable to connect to azure.archive.ubuntu.com:http: E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/p/pythonpy/python3py_1.5.21_all.deb Unable to connect to azure.archive.ubuntu.com:http: E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/p/pythonsetuptools/python3setuptools_39.0.12_all.deb Unable to connect to azure.archive.ubuntu.com:http: E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/universe/p/pythonvirtualenv/python3virtualenv_15.1.0+ds1.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/pythonvirtualenv/virtualenv_15.1.0+ds1.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.01_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/pythontox_2.5.01_all.deb Unable to connect to azure.archive.ubuntu.com:http: E: Unable to fetch some archives, maybe run aptget update or try with fixmissing?
 ubuntueoan, standardpython2
 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
 ubuntufocal, minimal
cancelled at
sage t src/sage/geometry/hyperplane_arrangement/check_freeness.py
 debianjessie, standard
 sage t src/sage/doctest/test.py # Timed out
sage: if system() == "Linux": P = subprocess.Popen(["sage", "t", "warnlong", "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/sagerelease/kU5M1QVuQQY/IEXj4PDAwAJ  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
 sage t src/sage/doctest/test.py # Timed out
 debianjessie, standardpython2
 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
 debianbuster, standard  debianbuster, standardpython2
 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
 debianbullseye, minimal
 sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
cancelled at
sage t src/sage/modular/modform/weight1.py
 sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
cancelled at
 debianbullseye, standard  debianbullseye, standardpython2  debiansid, 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 debiansid, standardpython2)
 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
 debiansid, minimal
cancelled at
sage t src/sage/misc/bindable_class.py
 linuxmint19.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
 fedora26, standardpython2
 fedora28, minimal
cancelled at
sage t src/sage/plot/disk.py
 centos7, minimal
 sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
cancelled at
sage t src/sage/sat/solvers/satsolver.pxd
 sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
cancelled at
 centos8, standard
cancelled during
[pplpy0.8.4] installing. Log file: /sage/logs/pkgs/pplpy0.8.4.log
 centos8, standardpython2
 sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
 centos8, minimal
cancelled at
sage t src/sage/manifolds/differentiable/characteristic_class.py
 centos8, standardpython2
cancelled during traceback of
sage t src/sage/manifolds/differentiable/degenerate_submanifold.py # Timed out
 archlinuxlatest, standard  archlinuxlatest, standardpython2
 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 ; followup: ↓ 35 Changed 2 years ago by
 Status changed from needs_review to needs_info
Replying to ghkliem:
Can someone please help me with the following issue.
 condaforge and homebrewmacos
`/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 2 years ago by
Replying to ghkliem:
Replying to ghkliem:
Can someone please help me with the following issue.
 condaforge and homebrewmacos
`/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 followup: ↓ 39 Changed 2 years ago by
Oh, I see, you added this on the ticket. See #29381.
comment:37 Changed 2 years ago by
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/sagerelease/kU5M1QVuQQY/5vDLmipuAwAJ
comment:38 Changed 2 years ago by
 Commit changed from 301bd2c279991115c5702b1837118339360e8e86 to 2d1d7980c413afc1e5c6625c16b3bac99daa7463
Branch pushed to git repo; I updated commit sha1. New commits:
2d1d798  testcflags.sh has moved to build/bin

comment:39 in reply to: ↑ 36 Changed 2 years ago by
comment:40 Changed 2 years ago by
 Status changed from needs_info to needs_review
Seems to work now:
https://github.com/kliem/sagetest27122/actions/runs/67557444
Some strange fail in (debianbullseye, standard):
cp: cannot create directory '/sage/local/./lib/python3.7/sitepackages/Cython/__pycache__': File exists
Looks somewhat like a racing condition to me. Given that it bullseye suceeds on minimal and standardpython2, 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. ubuntuxenial, 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
comment:41 Changed 2 years ago by
 Description modified (diff)
comment:42 Changed 2 years ago by
 Milestone set to sage9.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 2 years ago by
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 nondefault is more meaningful?
comment:44 Changed 2 years ago by
Isn't that what SAGE_FAT_BINARY
is for?
I just noticed that need to rebase.
comment:45 Changed 2 years ago by
 Branch changed from public/27122reb2 to public/27122reb3
 Commit changed from 2d1d7980c413afc1e5c6625c16b3bac99daa7463 to bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0
New commits:
24c1a87  compile with 'O2 march=native' if the user does not provide own flags

5ec3142  added 'g' as standard compile flag; 'O0' is standard in case of debugging now

8ca0b48  much simpler by using testcflags.sh

ecb7606  removed redundant flags O2, O0, g

5bdf4e4  remove flag Wall by default

4543249  adding comment about assuming compilers from same suite

bc6f7c1  testcflags.sh has moved to build/bin

comment:46 Changed 2 years ago by
 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 2 years ago by
Thanks.
comment:48 Changed 2 years ago by
Looks like @vbraun is already merging it for 9.1.
In a test run for ubuntutrustyminimal
(https://github.com/mkoeppe/sage/runs/617198833) I see a new error building gfortran:
/sage/local/var/tmp/sage/build/gfortran9.2.0/gccbuild/./gcc/xgcc B/sage/local/var/tmp/sage/build/gfortran9.2.0/gccbuild/./gcc/ B/sage/local/x86_64pclinuxgnu/bin/ B/sage/local/x86_64pclinuxgnu/lib/ isystem /sage/local/x86_64pclinuxgnu/include isystem /sage/local/x86_64pclinuxgnu/sysinclude O2 g march=native O2 O2 g march=native DIN_GCC W Wall Wnonarrowing Wwritestrings Wcastqual Wstrictprototypes Wmissingprototypes Woldstyledefinition isystem ./include fpic mlongdouble80 DUSE_ELF_SYMVER g DIN_LIBGCC2 fbuildinglibgcc fnostackprotector fpic mlongdouble80 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 (debianjessie: fflas_ffpack again) is from that run.
So this may need some more work
comment:49 Changed 2 years ago by
 Status changed from positive_review to needs_work
comment:50 Changed 2 years ago by
Also segfaults building dochtml with ubuntubionicstandard
(https://github.com/mkoeppe/sage/runs/617198861)
comment:51 Changed 2 years ago by
 Cc vbraun added
comment:52 followup: ↓ 59 Changed 2 years ago by
Also segfault on linuxmint19.3minimal
(https://github.com/mkoeppe/sage/runs/617198994)
Definitely not ready for 9.1.
comment:53 Changed 2 years ago by
Also breaks gfortran build on cygwinminimal
(https://github.com/mkoeppe/sage/runs/617198553)
comment:54 Changed 2 years ago by
 Branch changed from public/27122reb3 to bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0
 Resolution set to fixed
 Status changed from needs_work to closed
comment:55 Changed 2 years ago by
 Commit bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0 deleted
 Resolution fixed deleted
 Status changed from closed to new
comment:56 Changed 2 years ago by
 Branch changed from bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0 to public/27122reb3
 Commit set to bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0
New commits:
24c1a87  compile with 'O2 march=native' if the user does not provide own flags

5ec3142  added 'g' as standard compile flag; 'O0' is standard in case of debugging now

8ca0b48  much simpler by using testcflags.sh

ecb7606  removed redundant flags O2, O0, g

5bdf4e4  remove flag Wall by default

4543249  adding comment about assuming compilers from same suite

bc6f7c1  testcflags.sh has moved to build/bin

comment:57 Changed 2 years ago by
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 2 years ago by
This change is broken (empty then clause)
diff git a/build/pkgs/pari/spkgcheck.in b/build/pkgs/pari/spkgcheck.in index ae3aecb..a353b0e 100644  a/build/pkgs/pari/spkgcheck.in +++ b/build/pkgs/pari/spkgcheck.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 2 years ago by
Replying to mkoeppe:
Also segfault on
linuxmint19.3minimal
(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 2 years ago by
 Branch changed from public/27122reb3 to public/27122reb4
 Commit changed from bc6f7c1cf87e806b9f5ea1478bff0a2fc4cf23e0 to a04ca1e95e6af5f55e59be2832710eab5cd52c6b
comment:61 Changed 2 years ago by
 Commit changed from a04ca1e95e6af5f55e59be2832710eab5cd52c6b to 072e7e05ab233095c95bdbfb54b5bd984cc378d6
Branch pushed to git repo; I updated commit sha1. New commits:
072e7e0  typo

comment:62 Changed 2 years ago by
 Status changed from new to needs_info
It seems that my approaches for disabling march=native
for certain packages don't work.
For ubuntutrusty it tried to compile gfortran
with march=native
, but I tried to disable that.
https://github.com/kliem/sagetest27122/runs/622914460?check_suite_focus=true
For linuxmint19, 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/sagetest27122/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 years ago by
 Commit changed from 072e7e05ab233095c95bdbfb54b5bd984cc378d6 to 31db2e54c451c44b18234cfe839d5c6799e0922a
comment:64 Changed 2 years ago by
 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/sitepackages/sage_setup/docbuild/__main__.py", line 2, in <module> main() File "/sage/local/lib/python3.7/sitepackages/sage_setup/docbuild/__init__.py", line 1720, in main builder() File "/sage/local/lib/python3.7/sitepackages/sage_setup/docbuild/__init__.py", line 327, in _wrapper getattr(get_builder(document), 'inventory')(*args, **kwds) File "/sage/local/lib/python3.7/sitepackages/sage_setup/docbuild/__init__.py", line 552, in _wrapper self._build_everything_except_bibliography(lang, format, *args, **kwds) File "/sage/local/lib/python3.7/sitepackages/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/sitepackages/sage_setup/docbuild/__init__.py", line 280, in build_many _build_many(target, args, processes=NUM_THREADS) File "/sage/local/lib/python3.7/sitepackages/sage_setup/docbuild/utils.py", line 283, in build_many raise worker_exc.original_exception Exception: ('Nonexception 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/sagepackaging/VU4h8IWGFLA/rU8NCPjBgAJ
comment:65 followup: ↓ 70 Changed 2 years ago by
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 years ago by
likewise for SAGE_DEBUG
comment:67 Changed 2 years ago by
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 2 years ago by
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 2 years ago by
 Branch changed from public/27122reb4 to u/ghkliem/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:
f63aa17  add march=native in configure.ac

comment:70 in reply to: ↑ 65 ; followup: ↓ 75 Changed 2 years ago by
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 disablesse disablesse2 disablesse3 disablessse3 disablesse41 disablesse42 disablefma disablefma4 disableavx disableavx2
? However, I'm not sure that this will be used anywhere.
comment:71 Changed 2 years ago by
 Commit changed from f63aa174447b9b214fae82eb26727ddbea25f5b5 to 6e407f7b89dc1017a036edd466e81bc37d0ae91c
comment:72 Changed 2 years ago by
we configure compilers mostly in build/pkgs/{gcc,gfortran}/spkgconfigure.m4
 could you perhaps move (most of) your changes to configure.ac
there?
comment:73 followup: ↓ 74 Changed 2 years ago by
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 2 years ago by
Replying to ghkliem:
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/spkgconfigure.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 setCFLAGS
thenAC_PROG_CC()
will setCFLAGS
tog 02
(or different order).
AC_PROG_CC only is called early in configure.ac
to ensure general sanity.
As you might know, build/pkgs/*/spkgconfigure.m4
are collected by ./boostrap
and used to
generate ./configure
from them and configure.ac
. We have moved a lot of compilerspecific stuff into build/pkgs/*/spkgconfigure.m4
when the latter were instroduced, and for a good reason.
comment:75 in reply to: ↑ 70 ; followup: ↓ 77 Changed 2 years ago by
Replying to ghkliem:
Replying to mkoeppe:
I think determining flags should really be done in
configure
. Also let's add configure options for enabling SAGE_FAT_BINARYWhat kind of options should I add for this?
Something like
disablesse disablesse2 disablesse3 disablessse3 disablesse41 disablesse42 disablefma disablefma4 disableavx disableavx2
? However, I'm not sure that this will be used anywhere.
What I meant by this was for the toplevel configure to have an option enablefatbinary
(or perhaps a better name for that) so that this is not controlled by an environment variable.
comment:76 Changed 2 years ago by
 Commit changed from 6e407f7b89dc1017a036edd466e81bc37d0ae91c to 6ba54119a79ce724cccdc3d48c0661d1d7320af8
Branch pushed to git repo; I updated commit sha1. New commits:
6ba5411  enablefatbinary

comment:77 in reply to: ↑ 75 Changed 2 years ago by
Replying to mkoeppe:
Replying to ghkliem:
Replying to mkoeppe:
I think determining flags should really be done in
configure
. Also let's add configure options for enabling SAGE_FAT_BINARYWhat kind of options should I add for this?
Something like
disablesse disablesse2 disablesse3 disablessse3 disablesse41 disablesse42 disablefma disablefma4 disableavx disableavx2
? However, I'm not sure that this will be used anywhere.What I meant by this was for the toplevel configure to have an option
enablefatbinary
(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 2 years ago by
 Commit changed from 6ba54119a79ce724cccdc3d48c0661d1d7320af8 to a6b93f261b30adfdb37d8867dbb05e0713e32d8d
Branch pushed to git repo; I updated commit sha1. New commits:
a6b93f2  typo for iml

comment:79 Changed 2 years ago by
 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 debianjessie):
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
 cygwinstandard:
looks the same to me:
beta1: https://github.com/sagemath/sage/actions/runs/134966985
#27122: https://github.com/kliem/sage/actions/runs/136808676
 cygwinminimal:
looks the same to me:
beta1: https://github.com/sagemath/sage/actions/runs/134966984
comment:80 Changed 2 years ago by
 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 2 years ago by
 Commit changed from a6b93f261b30adfdb37d8867dbb05e0713e32d8d to c225c7ed5260b64dce8748c9aa92502874f2cd39
comment:82 Changed 2 years ago by
 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 2 years ago by
 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}/spkgconfigure.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 2 years ago by
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 followup: ↓ 86 Changed 2 years ago by
I think AC_PROG_CC
etc can be moved to gcc/gfortran's spkgconfigure.m4
comment:86 in reply to: ↑ 85 Changed 2 years ago by
Replying to dimpase:
I think
AC_PROG_CC
etc can be moved to gcc/gfortran'sspkgconfigure.m4
I did a very quick check, applying

configure.ac
a b AX_PROG_PERL_VERSION([5.8.0],[],[ 308 308 # Check C/C++/Fortran compilers 309 309 ############################################################################### 310 310 311 SAGE_SPKG_COLLECT() 311 312 SAGE_CHECK_CONDA_COMPILERS 312 313 313 314 AC_PROG_CC() … … AS_IF([test "x$enable_download_from_upstream_url" = "xyes"], [ 412 413 ]) 413 414 AC_SUBST([SAGE_SPKG_OPTIONS]) 414 415 415 SAGE_SPKG_COLLECT()416 416 417 417 # We need sageenvconfig to exist before running this. 418 418 # 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 2 years ago by
 Commit changed from c225c7ed5260b64dce8748c9aa92502874f2cd39 to 4596c4bcb5e9132c4e627edd8081ed139b22e613
comment:88 Changed 2 years ago by
 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 2 years ago by
One thing one could do, is to not build the documentation and hope that the problem shows up during the doctests.
comment:90 followup: ↓ 105 Changed 2 years ago by
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 sageongentoo 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 2 years ago by
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 spkginstall
. And as it uses the pari headers, it might recompile something and then lead to the problem.
comment:92 Changed 2 years ago by
And it wasn't cddlib
, at least not by itself.
comment:93 Changed 2 years ago by
 Work issues set to move to gcc/spkgconfigure; fix documentation build
comment:94 Changed 2 years ago by
 Status changed from needs_info to needs_review
I think I found the problem. After disabling gap, it worked.
comment:95 Changed 2 years ago by
 Commit changed from 4596c4bcb5e9132c4e627edd8081ed139b22e613 to 4c52d4d43794f807875739deb27a51d46836764c
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4c52d4d  gap without native

comment:96 Changed 2 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
 Work issues changed from move to gcc/spkgconfigure; fix documentation build to move to gcc/spkgconfigure; enabledebug; documententation for developers
I meant to change from "needs info" to "needs work" earlier.
comment:97 Changed 2 years ago by
 Commit changed from 4c52d4d43794f807875739deb27a51d46836764c to d12ed68a2a7a6fb9161cb11f066ca94c4b6c228e
comment:98 Changed 2 years ago by
 Status changed from needs_work to needs_review
 Work issues move to gcc/spkgconfigure; enabledebug; documententation for developers deleted
Tests are running here:
 tox.ini: https://github.com/kliem/sage/actions/runs/144837797
 tox.ini with gccspkg: https://github.com/kliem/sage/actions/runs/144837787
 optional: https://github.com/kliem/sage/actions/runs/144837791
 cygwin minimal: https://github.com/kliem/sage/actions/runs/144837775
 cygwin standard: https://github.com/kliem/sage/actions/runs/144837781
In comparison to 9.2beta1:
 tox.ini: https://github.com/sagemath/sage/actions/runs/134966983
 tox.ini with gccspkg: https://github.com/sagemath/sage/actions/runs/134966986
 optional: https://github.com/sagemath/sage/actions/runs/134966981
 cygwin minimal: https://github.com/sagemath/sage/actions/runs/134966984
 cygwin standard: https://github.com/sagemath/sage/actions/runs/134966985
comment:99 Changed 2 years ago by
Somehow it seems the second last commit 74b82b6506cd71d87f616389fe2fa142dd16f4cf
has messed up the scipy
build. Or something else is going on. I'm getting:
[sagelib9.2.beta1] installing. Log file: /sage/logs/pkgs/sagelib9.2.beta1.log [sagelib9.2.beta1] successfully installed. [scipy1.2.3] error installing, exit status 1. End of log file: [scipy1.2.3] Running setup.py install for scipy: finished with status 'error' [scipy1.2.3] Cleaning up... [scipy1.2.3] Removing source in /tmp/pipreqbuild18apw96c [scipy1.2.3] Removed build tracker '/tmp/pipreqtrackerx3hgraz6' [scipy1.2.3] Command "/sage/local/bin/python3 u c "import setuptools, tokenize;__file__='/tmp/pipreqbuild18apw96c/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" nousercfg install record /tmp/piprecordv0usfnl4/installrecord.txt singleversionexternallymanaged root /sage/local/var/tmp/sage/build/scipy1.2.3/inst compile installheaders /sage/local/include/site/python3.7/scipy" failed with error code 1 in /tmp/pipreqbuild18apw96c/ [scipy1.2.3] Exception information: [scipy1.2.3] Traceback (most recent call last): [scipy1.2.3] File "/sage/local/lib/python3.7/sitepackages/pip/_internal/cli/base_command.py", line 143, in main [scipy1.2.3] status = self.run(options, args) [scipy1.2.3] File "/sage/local/lib/python3.7/sitepackages/pip/_internal/commands/install.py", line 366, in run [scipy1.2.3] use_user_site=options.use_user_site, [scipy1.2.3] File "/sage/local/lib/python3.7/sitepackages/pip/_internal/req/__init__.py", line 49, in install_given_reqs [scipy1.2.3] **kwargs [scipy1.2.3] File "/sage/local/lib/python3.7/sitepackages/pip/_internal/req/req_install.py", line 791, in install [scipy1.2.3] spinner=spinner, [scipy1.2.3] File "/sage/local/lib/python3.7/sitepackages/pip/_internal/utils/misc.py", line 705, in call_subprocess [scipy1.2.3] % (command_desc, proc.returncode, cwd)) [scipy1.2.3] pip._internal.exceptions.InstallationError: Command "/sage/local/bin/python3 u c "import setuptools, tokenize;__file__='/tmp/pipreqbuild18apw96c/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" nousercfg install record /tmp/piprecordv0usfnl4/installrecord.txt singleversionexternallymanaged root /sage/local/var/tmp/sage/build/scipy1.2.3/inst compile installheaders /sage/local/include/site/python3.7/scipy" failed with error code 1 in /tmp/pipreqbuild18apw96c/ [scipy1.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}/spkgconfigure.ac
.
comment:100 Changed 2 years ago by
 Commit changed from d12ed68a2a7a6fb9161cb11f066ca94c4b6c228e to 16a6989274d87c44815f05dab041c5ea825db759
Branch pushed to git repo; I updated commit sha1. New commits:
16a6989  partly undo reordering

comment:101 Changed 2 years ago by
I undid part of the reordering (or most of it).
It looks much better now (some tests still running):
 tox.ini: https://github.com/kliem/sage/actions/runs/145781781
 gccspkg: https://github.com/kliem/sage/actions/runs/145781778
 optional: https://github.com/kliem/sage/actions/runs/145781780
 cygwin minimal: https://github.com/kliem/sage/actions/runs/145781783
 cygwin standard: https://github.com/kliem/sage/actions/runs/145781785
comment:102 Changed 2 years ago by
OK, this still looks acceptable, w.r.t. what is done in configure.ac, and try further refactoring on another ticket.
comment:103 Changed 2 years ago by
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 2 years ago by
Here is a overview, over the test results (all in comparison to the 9.2.beta1 tests and only mentioning differences):
 tox.ini:
 ubuntutrustystandard
sage t src/sage/interfaces/maxima.py # Timed out
aftersage: maxima._batch('10003;') ## line 864 ## 'kill(sage121)$batchload("/root/.sage/temp/9b341214822c/14051/interface/tmp140541371013559");1+952962022;\r\n\r\n(%o771) "/root/.sage/temp/9b341214822c/14051/interface/tmp140541371013559"\r\n(%o772) ' sage: maxima._batch('10003;',batchload=False) ## line 866 ##
 debianstretchstandard: didn't finish in 6 hours
 linuxmint19.3minimal: didn't finish in 6 hours
 fedora26, standard:
sage t src/sage/symbolic/expression.pyx # 1 doctest failed
 fedora27, minimal: failed to build
mpir
 fedora28, standard:
sage t src/sage/interfaces/maxima.py # Timed out
 fedora31, minimal:
sage t src/sage/symbolic/integration/integral.py # 1 doctest failed
7  fedora32, standard didn't finish in time and
sage t src/sage/interfaces/maxima.py # Timed out
 localmacos (homebrewmacospython3_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
 localmacos (homebrewmacospython3_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
 localmacos (homebrewmacospython3_xcodenokegonly, minimal)
sage t src/sage/plot/plot3d/shapes.pyx # Timed out sage t src/sage/rings/function_field/function_field.py # Timed out
 localmacos (homebrewmacospython3_xcodenokegonly, standard)
sage t src/sage/plot/plot3d/shapes.pyx # Timed out sage t src/sage/rings/function_field/function_field.py # Timed out
 localmacos (homebrewmacospython3_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
 localmacos (homebrewmacospython3_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
 localmacos (condaforgemacos): many failures, looks like the new ones are the same as other mac
 ubuntutrustystandard
I'll leave it at that for now.
All those new mac failures don't look good.
comment:105 in reply to: ↑ 90 Changed 2 years ago by
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 sageongentoo builds and I am fine. Butpari
is definitely sensitive to optimisation. When I was working on enabling clang, I also tried the Intel compiler.pari
needed to be compiled atO0
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 2 years ago by
 Commit changed from 16a6989274d87c44815f05dab041c5ea825db759 to 863862679f6a9cba3476b515dbbfe22ed31a6571
comment:107 followup: ↓ 108 Changed 2 years ago by
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:
 tox.ini https://github.com/kliem/sage/actions/runs/147321660
 tox.ini gccspkg: https://github.com/kliem/sage/actions/runs/147321678
 targets optional: https://github.com/kliem/sage/actions/runs/147321655
 cygwin minimal: https://github.com/kliem/sage/actions/runs/147321676
 cygwin standard: https://github.com/kliem/sage/actions/runs/147321676
comment:108 in reply to: ↑ 107 Changed 2 years ago by
Replying to ghkliem:
maxima
doesn't compile withmarch=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 years ago by
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):
 localmacos (homebrewmacospython3_pythonorg, minimal) https://github.com/mkoeppe/sage/runs/785733405?check_suite_focus=true
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 homebrewmacospython3_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
forpari
, 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 years ago by
 Commit changed from 863862679f6a9cba3476b515dbbfe22ed31a6571 to 28ac47550eebe0ad6cd3094b432bed29085838ff
comment:111 Changed 2 years ago by
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:
localmacos (homebrewmacospython3_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 years ago by
Could you comment on whether this implements autotools semantics of "precious variables" (see #29507) for CFLAGS, FCFLAGS etc.?
comment:113 Changed 2 years ago by
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 sagebuildenvconfig.in
does). They are also mostly respected now. Until now, if you set CFLAGS
to O0
, every package spkginstall.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 years ago by
 Commit changed from 28ac47550eebe0ad6cd3094b432bed29085838ff to 234338ea6283549195c4b22fc84cb621fcb75c48
Branch pushed to git repo; I updated commit sha1. New commits:
234338e  export SAGE_DEBUG, if this was enabled during configure

comment:115 Changed 2 years ago by
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 2 years ago by
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 2 years ago by
+AC_ARG_ENABLE([debug],
+ [AS_HELP_STRING([enabledebug={nosymbolsyes}],
+ [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 enabledebug=...
, it will override this variable.
Note also that this must now be done at configuration time.
I just checked. The following all work:
./configure enabledebug=yes
./configure SAGE_DEBUG=yes
export SAGE_DEBUG=yes; ./configure
comment:118 followup: ↓ 119 Changed 2 years ago by
I think it would be good if at least make SAGE_DEBUG=yes somepackage
would still be supported.
comment:119 in reply to: ↑ 118 Changed 2 years ago by
Replying to mkoeppe:
I think it would be good if at least
make SAGE_DEBUG=yes somepackage
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 sagebuildenvconfig.in
. There we could do
export SAGE_DEBUG?="@SAGE_DEBUG@"
and then build together all the flags according to SAGE_DEBUG
.
comment:120 Changed 2 years ago by
This sounds like a good solution.
I think it's important to support perpackage SAGE_DEBUG, just like we support perpackage SAGE_CHECK.
comment:121 Changed 2 years ago by
 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  enabledebug as configuration option

6fd7ba7  move to buil/spkg/{gcc,gfortran}/spkgconfigure.ac

03b002a  disable native for gcc build

0660ab0  partly undo reordering

5eba95a  pari without native

1102ac1  lcalc for real

d412f06  Revert "lcalc for real"

453c97b  Revert "pari without native"

f23e68c  export SAGE_DEBUG, if this was enabled during configure

e9fb1a3  allow debuggin of individual packages

comment:122 Changed 2 years ago by
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 somepackage
. 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 2 years ago by
 Description modified (diff)
comment:124 Changed 2 years ago by
 Commit changed from e9fb1a37af6ac08c29dff5a9cb062eccf60f761a to df59361cd348455c9280779bc256f5903328ff75
Branch pushed to git repo; I updated commit sha1. New commits:
df59361  merge u/ghkliem/march_native_in_configure

comment:125 Changed 2 years ago by
 Milestone changed from sage9.2 to sage9.3
I would be glad if we could bring this in, in an early 9.3 beta.
One could also think of splitting this ticket into two. One that reorders the all the flags and one where we enable march=native.
comment:126 followup: ↓ 129 Changed 2 years ago by
Yes, I agree with both points.
comment:127 Changed 2 years ago by
 Commit changed from df59361cd348455c9280779bc256f5903328ff75 to 5da42f16bf3d470bbc6e42beaa3e7485650a3f36
comment:128 Changed 2 years ago by
 Dependencies set to #30375
 Description modified (diff)
comment:129 in reply to: ↑ 126 Changed 2 years ago by
Replying to mkoeppe:
Yes, I agree with both points.
Ok. The controversial part of adding march=native
is now down to a rather small commit.
comment:130 Changed 20 months ago by
 Status changed from needs_review to needs_work
 Work issues set to Merge latest branch of dependencies
comment:131 Changed 20 months ago by
 Branch changed from u/ghkliem/march_native_in_configure to u/ghkliem/march_native_on_30375
 Commit changed from 5da42f16bf3d470bbc6e42beaa3e7485650a3f36 to 4e6273441de2114a7a9ea36a69a3137976a1bfa2
 Status changed from needs_work to needs_review
Last 10 new commits:
431d14c  source in sagespkg

9c5e8a2  add references to `bin/sagebuildenv` and `bin/sagebuildenvconfig`

4b8933b  remove incorrect `$` in `build/bin/sagebuildenv`

071b8cc  Revert "add references to `bin/sagebuildenv` and `bin/sagebuildenvconfig`"

c51e752  correct comments to makefile

9a59627  check whether gcc accepts "march=native"

dc711ec  use CONFIGURED_MARCH_NATIVE whenever suitable

64d2ee1  fix that we might have changed $FCFLAGS during configure

800d04b  documentation

4e62734  no march=native for eclib, gap, gcc, mpfr and r

comment:132 Changed 20 months ago by
 Commit changed from 4e6273441de2114a7a9ea36a69a3137976a1bfa2 to a1ffd9b1c5ee9689ad628ebbe48847a495e1c0d1
comment:133 Changed 20 months ago by
 Commit changed from a1ffd9b1c5ee9689ad628ebbe48847a495e1c0d1 to ea601ea6742ddcfab610aeb7cc53fec375d77c7a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a29103d  check whether gcc accepts "march=native"

339a46e  use CONFIGURED_MARCH_NATIVE whenever suitable

e4d331a  fix that we might have changed $FCFLAGS during configure

e3e5aaa  documentation

ea601ea  no march=native for eclib, gap, gcc, mpfr and r

comment:134 Changed 20 months ago by
 Reviewers changed from Matthias Koeppe to Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861
 Work issues Merge latest branch of dependencies deleted
comment:135 followup: ↓ 137 Changed 20 months ago by
Instead of SAGE_MARCH
, perhaps something like CFLAGS_MARCH
...?!
And I would suggest to not use the prefix CONFIGURED_
here  because I think this is not a case as for CFLAGS where we want to respect user variable settings?
comment:136 Changed 20 months ago by
 Commit changed from ea601ea6742ddcfab610aeb7cc53fec375d77c7a to 73ecf70a1ecfa0be713378bcc3c674804a908e57
Branch pushed to git repo; I updated commit sha1. New commits:
73ecf70  better naming scheme

comment:137 in reply to: ↑ 135 Changed 20 months ago by
Replying to mkoeppe:
Instead of
SAGE_MARCH
, perhaps something likeCFLAGS_MARCH
...?! And I would suggest to not use the prefixCONFIGURED_
here  because I think this is not a case as for CFLAGS where we want to respect user variable settings?
Sure.
comment:138 followup: ↓ 140 Changed 20 months ago by
And I thought you had found a better solution for that freeform
business in #30375?
comment:139 Changed 20 months ago by
 Commit changed from 73ecf70a1ecfa0be713378bcc3c674804a908e57 to 7bc1f285f9256ebe8f5749848bfd3b97bc173853
Branch pushed to git repo; I updated commit sha1. New commits:
7bc1f28  remove redundant freeform patch

comment:140 in reply to: ↑ 138 Changed 20 months ago by
comment:141 followup: ↓ 143 Changed 20 months ago by
Looks good to me. If you have capacity on GH Actions, could you run it one more time? (Ideally with #31064 for the latest version of the cygwin workflow)
comment:142 Changed 20 months ago by
 Reviewers changed from Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861 to Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861, https://github.com/kliem/sage/pull/30/checks
comment:143 in reply to: ↑ 141 Changed 20 months ago by
comment:144 followup: ↓ 146 Changed 20 months ago by
 Reviewers changed from Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861, https://github.com/kliem/sage/pull/30/checks to Matthias Koeppe
 Status changed from needs_review to positive_review
Looking good. Thanks for your patience with this long review process.
comment:145 Changed 20 months ago by
 Branch changed from u/ghkliem/march_native_on_30375 to 7bc1f285f9256ebe8f5749848bfd3b97bc173853
 Resolution set to fixed
 Status changed from positive_review to closed
comment:146 in reply to: ↑ 144 Changed 20 months ago by
 Commit 7bc1f285f9256ebe8f5749848bfd3b97bc173853 deleted
Replying to mkoeppe:
Looking good. Thanks for your patience with this long review process.
Thank you. And thank you in particular for creating the github workflow for testing.
The topic has nothing to do with polyhedra, many parts of Sage could benefit from it.