search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.41k stars 475 forks source link

Upgrade to lcalc-2.0.4 #32037

Closed orlitzky closed 2 years ago

orlitzky commented 3 years ago

The new lcalc 2.0.3 has been released. Note in particular:

In this ticket we upgrade to lcalc 2.0.3 and:

Depends on #32061 Depends on #31837

CC: @JohnCremona @antonio-rojas @kiwifb @dimpase

Component: packages: standard

Author: Michael Orlitzky, Frédéric Chapoton

Branch: cbb7908

Reviewer: Antonio Rojas, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/32037

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from dd656cf to 93f9928

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

9a42b78Trac #32037: delete a failing doctest.
1537168Trac #32037: replace some ellipses with abs_tol.
c2f3e5cTrac #32037: adjust doctest tolerance for lcalc-2.0.1 in lseries_ell.py.
5a63aa5Trac #32037: weaken doctest tolerance for lcalc-2.0.1 in modular/dirichlet.py.
68b89a7Trac #32037: tweak a doctest in elliptic_curves/ell_rational_field.py.
9bb551bTrac #32037: update to lcalc-2.0.2 to fix the build with std=c++11.
513003cTrac #32037: add tolerance to a test in sage/lfunctions/zero_sums.pyx.
413f114Trac #32037: upgrade to lcalc-2.0.3.
3ea32f7Trac #32037: make getgetopt an order-only dependency of lcalc.
93f9928Trac #32037: remove remaining lcalc patches.
orlitzky commented 2 years ago
comment:75

Rebased cleanly onto develop, and added one additional commit to remove the new patch.

fchapoton commented 2 years ago
comment:76

red branch => needs work

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

1a4d43dTrac #32037: update the doctest for the DirichletGroup(5)[1] L-function.
1601d3bTrac #32037: lower all lcalc tolerances to 1e-8.
7ab32bdTrac #32037: add abs_tol to an lcalc test to fix a new failure.
2897402Trac #32037: update an lcalc doctest that counts zeros.
c44adc5Trac #32037: add tolerance to a few sage/lfunctions/lcalc.py doctests.
ef57b43Trac #32037: delete a failing doctest.
ad5c3bfTrac #32037: replace some ellipses with abs_tol.
fae1d20Trac #32037: adjust doctest tolerance for lcalc-2.0.1 in lseries_ell.py.
35870f2Trac #32037: weaken doctest tolerance for lcalc-2.0.1 in modular/dirichlet.py.
69e646dTrac #32037: tweak a doctest in elliptic_curves/ell_rational_field.py.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 93f9928 to 69e646d

orlitzky commented 2 years ago
comment:78

Not sure why, it merged cleanly with beta6 for me. Here it is again rebased onto develop.

dimpase commented 2 years ago

Changed reviewer from Antonio Rojas to Antonio Rojas, Dima Pasechnik

dimpase commented 2 years ago
comment:79

lgtm

vbraun commented 2 years ago
comment:80
Using cached file /home/release/Sage/upstream/lcalc-2.0.1.tar.xz
lcalc-2.0.1
====================================================
Setting up build directory for lcalc-2.0.1
Finished extraction
Applying patches from ../patches...
Applying ../patches/lcalc-1.23-gcc11.patch
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/include/Lcommon.h b/include/Lcommon.h
|index 1b3be43..717fcde 100644
|--- a/include/Lcommon.h
|+++ b/include/Lcommon.h
--------------------------
File to patch: 
Skip this patch? [y] 
Skipping patch.
2 out of 2 hunks ignored
Error applying '../patches/lcalc-1.23-gcc11.patch'
************************************************************************
Error applying patches
************************************************************************
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 69e646d to 8286107

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

eb84c7bTrac #32037: delete a failing doctest.
648e131Trac #32037: replace some ellipses with abs_tol.
f80d335Trac #32037: adjust doctest tolerance for lcalc-2.0.1 in lseries_ell.py.
dc5766cTrac #32037: weaken doctest tolerance for lcalc-2.0.1 in modular/dirichlet.py.
efabc7eTrac #32037: tweak a doctest in elliptic_curves/ell_rational_field.py.
7b0f6e2Trac #32037: update to lcalc-2.0.2 to fix the build with std=c++11.
8a2e9d8Trac #32037: add tolerance to a test in sage/lfunctions/zero_sums.pyx.
e00ab0cTrac #32037: upgrade to lcalc-2.0.3.
a601195Trac #32037: make getgetopt an order-only dependency of lcalc.
8286107Trac #32037: remove remaining lcalc patches.
vbraun commented 2 years ago
comment:83

stil same error?

vbraun commented 2 years ago
comment:84

nevermind...

vbraun commented 2 years ago
comment:85

On Debian 9 x86_64:

**********************************************************************
File "src/sage/libs/lcalc/lcalc_Lfunction.pyx", line 194, in sage.libs.lcalc.lcalc_Lfunction.Lfunction.hardy_z_function
Failed example:
    L.hardy_z_function(0) # abs tol 1e-8
Expected:
    0.793967590477 + 0.0*I
Got:
    0.793967590477090
**********************************************************************
1 item had failures:
   1 of  14 in sage.libs.lcalc.lcalc_Lfunction.Lfunction.hardy_z_function
    [105 tests, 1 failure, 0.76 s]
----------------------------------------------------------------------
sage -t --long --random-seed=234205826713938282410851315937822845647 src/sage/libs/lcalc/lcalc_Lfunction.pyx  # 1 doctest failed
----------------------------------------------------------------------

on OSX Big Sur:

In file included from ../../src/libLfunction/L.h:43:
In file included from ../../src/libLfunction/Lglobals.h:55:
../../src/libLfunction/Lcomplex.h:48:10: fatal error: 'bits/c++config.h' file not found
#include <bits/c++config.h>
         ^~~~~~~~~~~~~~~~~~
1 error generated.
make[6]: *** [Lcommandline.o] Error 1
make[6]: Target `all' not remade because of errors.
Making all in tests
Making all in lib
make[7]: Nothing to be done for `all'.
make[7]: Nothing to be done for `all-am'.
make[6]: Nothing to be done for `all-am'.
make[5]: *** [all-recursive] Error 1
make[5]: Target `all' not remade because of errors.
********************************************************************************
Error building lcalc-2.0.3
********************************************************************************
orlitzky commented 2 years ago
comment:86

Replying to @vbraun:

On Debian 9 x86_64:

**********************************************************************
File "src/sage/libs/lcalc/lcalc_Lfunction.pyx", line 194, in sage.libs.lcalc.lcalc_Lfunction.Lfunction.hardy_z_function
Failed example:
    L.hardy_z_function(0) # abs tol 1e-8
Expected:
    0.793967590477 + 0.0*I
Got:
    0.793967590477090
**********************************************************************

Well... all of these abs_tol tests involving complex numbers are wrong. I was naively assuming that our doctest framework would do the obvious thing here, but it's using regexps to find strings that look like floating point numbers and then e.g. deciding that the lists ["0.793967590477", " + 0.0"] and ["0.793967590477090"] cannot be equivalent because they are of different lengths.

I'll redo all of the tests to compare the real and imaginary parts separately I guess.

on OSX Big Sur:

In file included from ../../src/libLfunction/L.h:43:
In file included from ../../src/libLfunction/Lglobals.h:55:
../../src/libLfunction/Lcomplex.h:48:10: fatal error: 'bits/c++config.h' file not found
#include <bits/c++config.h>
         ^~~~~~~~~~~~~~~~~~

I don't know what to do about this. That's a standard C++ header, and I don't have access to OSX to debug. The full build log might help, if something is very obviously wrong.

kiwifb commented 2 years ago
comment:87

Seen that kind of stuff before when porting to clang. I don't think anything in bits should be included directly because they are implementation specific functions. Would have to check what exactly was needed from there.

From the file itself

/** @file bits/c++config.h
  *  This is an internal header file, included by other library headers.
  *  Do not attempt to use it directly. @headername{iosfwd}
  */
kiwifb commented 2 years ago
comment:88

For gcc, this particular header is included in <cmath> which is also included later. So, I'd recommend removing that include altogether.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

22efbb4Trac #32037: don't use "abs tol" for complex numbers.
acf4d44Trac #32037: whitespace cleanup in sage.libs.lcalc.lcalc_Lfunction.
1a2dd14Trac #32037: example cosmetology in sage.libs.lcalc.lcalc_Lfunction.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 8286107 to 1a2dd14

orlitzky commented 2 years ago
comment:90

Replying to @kiwifb:

For gcc, this particular header is included in <cmath> which is also included later. So, I'd recommend removing that include altogether.

This works well with both clang and gcc on linux... but can anyone confirm that it works on OSX before I make a whole new release and upgrade again?

mkoeppe commented 2 years ago
comment:91

If the repo was on GH, I'd send you a PR for portability/integration testing.

Looks like GitLab is also starting to provide macOS testing? https://docs.gitlab.com/ee/ci/runners/saas/macos_saas_runner.html

kiwifb commented 2 years ago
comment:92

Replying to @orlitzky:

Replying to @kiwifb:

For gcc, this particular header is included in <cmath> which is also included later. So, I'd recommend removing that include altogether.

This works well with both clang and gcc on linux... but can anyone confirm that it works on OSX before I make a whole new release and upgrade again?

Could have mentioned that. But clang on linux is not a good test unless you build clang's libstdc++ and make use of it. However, if you do it almost a sure deal. But I couldn't test on mac os because I am not properly set and was missing dependencies.

dimpase commented 2 years ago
comment:93

clang's C++ library is libc++, not libstdc++

I'll try checking on macOS 12 now.

dimpase commented 2 years ago
comment:94
libtool: compile:  clang++ -std=gnu++11 -DHAVE_CONFIG_H -I. -I../../src -g -O2 -c Lriemannsiegel.cc  -fno-common -DPIC -o .libs/Lriemannsiegel.o
In file included from Lmisc.cc:24:
In file included from ./Lmisc.h:27:
In file included from ./Lglobals.h:55:
./Lcomplex.h:48:10: fatal error: 'bits/c++config.h' file not found
In file included from Lgamma.cc:24:
In file included from ./Lgamma.h:#include <bits/c++config.h>
         ^~~~~~~~~~~~~~~~~~
28:
In file included from ./Lglobals.h:55:
./Lcomplex.h:48:10: fatal error: 'bits/c++config.h' file not found
#include <bits/c++config.h>
         ^~~~~~~~~~~~~~~~~~
In file included from Lglobals.cc:24:
In file included from ./Lglobals.h:55:
./Lcomplex.h:48:10: fatal error: 'bits/c++config.h' file not found
#include <bits/c++config.h>
         ^~~~~~~~~~~~~~~~~~
1 error generated.
In file included from Lnumbertheory.cc:24:
In file included from ./Lnumbertheory.h:32:
In file included from ./Lglobals.h:55:
./Lcomplex.h:48:10: fatal error: 'bits/c++config.h' file not found
#include <bits/c++config.h>
         ^~~~~~~~~~~~~~~~~~
make[6]: *** [Lmisc.lo] Error 1
make[6]: *** Waiting for unfinished jobs....

removing the offending #include, i.e.

diff --git a/src/libLfunction/Lcomplex.h b/src/libLfunction/Lcomplex.h
index 639dbfa..be29aab 100644
--- a/src/libLfunction/Lcomplex.h
+++ b/src/libLfunction/Lcomplex.h
@@ -45,7 +45,7 @@

 #pragma GCC system_header

-#include <bits/c++config.h>
+// #include <bits/c++config.h>

 //no longer include:
 //#include <bits/cpp_type_traits.h>  only thing used was is_floating...

makes it build

orlitzky commented 2 years ago
comment:95

Thanks, release/upgrade incoming.

orlitzky commented 2 years ago
comment:96

Replying to @kiwifb:

Could have mentioned that. But clang on linux is not a good test unless you build clang's libstdc++ and make use of it.

Argh, TIL about USE=default-libcxx on sys-devel/clang.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

cbb7908Trac #32037: upgrade to lcalc-2.0.4.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 1a2dd14 to cbb7908

orlitzky commented 2 years ago
comment:98

Certain (non-sage) builds were failing with newer clangs anyway so I guess this was timely.

dimpase commented 2 years ago
comment:99

maybe you can drop all the register things, as

 warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]

is all over there on macOS.

dimpase commented 2 years ago
comment:100

lgtm, otherwise

orlitzky commented 2 years ago
comment:101

Replying to @dimpase:

maybe you can drop all the register things, as

 warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]

is all over there on macOS.

Are you sure this was with v2.0.4? I already fixed a warning like that, and there's no more register in the lcalc repo.

dimpase commented 2 years ago
comment:102

Ah, these came from the included pari headers /usr/local/include/pari/pari.h. All good.

vbraun commented 2 years ago

Changed branch from u/mjo/ticket/32037 to cbb7908

mkoeppe commented 2 years ago

Changed commit from cbb7908 to none

mkoeppe commented 2 years ago
comment:104

This broke cygwin: #33043