Github messages for voidlinux
 help / color / mirror / Atom feed
* [PR PATCH] sympow: on musl, do not mess with fpu control word
@ 2021-11-22 23:43 tornaria
  2021-11-22 23:50 ` q66
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: tornaria @ 2021-11-22 23:43 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

There is a new pull request by tornaria against master on the void-packages repository

https://github.com/tornaria/void-packages sympow
https://github.com/void-linux/void-packages/pull/34209

sympow: on musl, do not mess with fpu control word
It turns out doing that results in broken precision, and not doing it
fixes it.

A simple way to test it is to run

    $ sympow -curve '[0,0,0,0,1]' -analrank
    ...
    Analytic Rank is 0 : L-value 7.01091e-01

When broken this prints "7.01092e-01" instead.

The actual value computed to 128 bits with pari is:

    $ echo 'elllseries(ellinit([0,0,0,0,1]),1)' | gp -q
    0.70109105266272713058750953952514706773

so the glibc output is the correct one.

It seems to me that what is broken is not computing but printing, but
this is important since sagemath parses sympow output.

This affects sagemath doctests as in

    $ sage -t src/sage/lfunctions/sympow.py
    ...
    (3 failures)

After this commit, the doctest in question passes.

<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **YES**

The corresponding doctest in sagemath passes on musl with this change; it was failing before.

A patch file from https://github.com/void-linux/void-packages/pull/34209.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-sympow-34209.patch --]
[-- Type: text/x-diff, Size: 1766 bytes --]

From ac4607e56eca77119b40f2d71275bbc812234f43 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gonzalo=20Tornar=C3=ADa?= <tornaria@cmat.edu.uy>
Date: Mon, 22 Nov 2021 19:24:50 -0300
Subject: [PATCH] sympow: on musl, do not mess with fpu control word

It turns out doing that results in broken precision, and not doing it
fixes it.

A simple way to test it is to run

    $ sympow -curve '[0,0,0,0,1]' -analrank
    ...
    Analytic Rank is 0 : L-value 7.01091e-01

When broken this prints "7.01092e-01" instead.

The actual value computed to 128 bits with pari is:

    $ echo 'elllseries(ellinit([0,0,0,0,1]),1)' | gp -q
    0.70109105266272713058750953952514706773

so the glibc output is the correct one.

It seems to me that what is broken is not computing but printing, but
this is important since sagemath parses sympow output.

This affects sagemath doctests as in

    $ sage -t src/sage/lfunctions/sympow.py
    ...
    (3 failures)

After this commit, the doctest in question passes.
---
 srcpkgs/sympow/template | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/srcpkgs/sympow/template b/srcpkgs/sympow/template
index d484cf847f57..9a8532ad8a70 100644
--- a/srcpkgs/sympow/template
+++ b/srcpkgs/sympow/template
@@ -1,7 +1,7 @@
 # Template file for 'sympow'
 pkgname=sympow
 version=2.023.6
-revision=2
+revision=3
 wrksrc=${pkgname}-v${version}
 build_style=configure
 make_build_target=all
@@ -16,6 +16,11 @@ checksum=d153530dfdd46da05c954121640e50771064536fedc22c7fef24fb11083172ef
 nocross=yes # runs binaries built for target
 
 do_configure() {
+	case "$XBPS_TARGET_MACHINE" in
+		# changing the fpu control word breaks under musl 64 bit (?)
+		*-musl) sed -i -e 's|fpu_53bits();||' main.c ;;
+	esac
+
 	PREFIX=/usr ./Configure
 }
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: sympow: on musl, do not mess with fpu control word
  2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
@ 2021-11-22 23:50 ` q66
  2021-11-23  2:28 ` [PR PATCH] [Updated] " tornaria
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: q66 @ 2021-11-22 23:50 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 156 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/34209#issuecomment-976014126

Comment:
make a proper patch

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PR PATCH] [Updated] sympow: on musl, do not mess with fpu control word
  2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
  2021-11-22 23:50 ` q66
@ 2021-11-23  2:28 ` tornaria
  2021-11-23  2:31 ` tornaria
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: tornaria @ 2021-11-23  2:28 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

There is an updated pull request by tornaria against master on the void-packages repository

https://github.com/tornaria/void-packages sympow
https://github.com/void-linux/void-packages/pull/34209

sympow: on musl, do not mess with fpu control word
It turns out doing that results in broken precision, and not doing it
fixes it.

A simple way to test it is to run

    $ sympow -curve '[0,0,0,0,1]' -analrank
    ...
    Analytic Rank is 0 : L-value 7.01091e-01

When broken this prints "7.01092e-01" instead.

The actual value computed to 128 bits with pari is:

    $ echo 'elllseries(ellinit([0,0,0,0,1]),1)' | gp -q
    0.70109105266272713058750953952514706773

so the glibc output is the correct one.

It seems to me that what is broken is not computing but printing, but
this is important since sagemath parses sympow output.

This affects sagemath doctests as in

    $ sage -t src/sage/lfunctions/sympow.py
    ...
    (3 failures)

After this commit, the doctest in question passes.

<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **YES**

The corresponding doctest in sagemath passes on musl with this change; it was failing before.

A patch file from https://github.com/void-linux/void-packages/pull/34209.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-sympow-34209.patch --]
[-- Type: text/x-diff, Size: 2675 bytes --]

From e483ed0e729742ec8eb83eb81d0f7007ad95708e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gonzalo=20Tornar=C3=ADa?= <tornaria@cmat.edu.uy>
Date: Mon, 22 Nov 2021 19:24:50 -0300
Subject: [PATCH] sympow: do not mess with fpu control word

It turns out doing that on musl results in wrong output for doubles.
Moreover any cpu with sse2 should not be using fp registers anyway.

For i686 without sse, the configure will figure out that floats are
80 bits and add -ffloat-store to CFLAGS to force them to be 64 bits.

A simple way to test it is to run

    $ sympow -curve '[0,0,0,0,1]' -analrank
    ...
    Analytic Rank is 0 : L-value 7.01091e-01

When broken this prints "7.01092e-01" instead.

The actual value computed to 128 bits with pari is:

    $ echo 'elllseries(ellinit([0,0,0,0,1]),1)' | gp -q
    0.70109105266272713058750953952514706773

so the glibc output is the correct one.

What is broken is not computing but printing, as fmt_fp in musl uses
long double, which means messing with fpu control word breaks it.
This is important since sagemath parses sympow output.

This affects sagemath doctests as in

    $ sage -t src/sage/lfunctions/sympow.py
    ...
    (3 failures)

After this commit, the doctest in question passes.
---
 .../patches/dont-change-fpu-control-word.patch   | 16 ++++++++++++++++
 srcpkgs/sympow/template                          |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 srcpkgs/sympow/patches/dont-change-fpu-control-word.patch

diff --git a/srcpkgs/sympow/patches/dont-change-fpu-control-word.patch b/srcpkgs/sympow/patches/dont-change-fpu-control-word.patch
new file mode 100644
index 000000000000..2a1155111268
--- /dev/null
+++ b/srcpkgs/sympow/patches/dont-change-fpu-control-word.patch
@@ -0,0 +1,16 @@
+--- a/Configure	2021-11-22 22:20:42.512565997 -0300
++++ b/Configure	2021-11-22 23:10:09.986457874 -0300
+@@ -154,9 +154,10 @@
+ done
+ 
+ # Select the most appropriate FPU control scheme
+-for FLAG in '-DISOC99_FENV' '-DFPUCONTROLH' '-Dx86'; do
+-    try_add_CFLAG $FLAG && break
+-done
++## disable messing with fpu control word -- not worth it
++# for FLAG in '-DISOC99_FENV' '-DFPUCONTROLH' '-Dx86'; do
++#     try_add_CFLAG $FLAG && break
++# done
+ 
+ # Some flags to try as last resort.  These hurt performance, so only add
+ # them if needed.
diff --git a/srcpkgs/sympow/template b/srcpkgs/sympow/template
index d484cf847f57..48e2399b72e3 100644
--- a/srcpkgs/sympow/template
+++ b/srcpkgs/sympow/template
@@ -1,7 +1,7 @@
 # Template file for 'sympow'
 pkgname=sympow
 version=2.023.6
-revision=2
+revision=3
 wrksrc=${pkgname}-v${version}
 build_style=configure
 make_build_target=all

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: sympow: on musl, do not mess with fpu control word
  2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
  2021-11-22 23:50 ` q66
  2021-11-23  2:28 ` [PR PATCH] [Updated] " tornaria
@ 2021-11-23  2:31 ` tornaria
  2021-11-23 12:00 ` [WIP] " tornaria
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: tornaria @ 2021-11-23  2:31 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

New comment by tornaria on void-packages repository

https://github.com/void-linux/void-packages/pull/34209#issuecomment-976109324

Comment:
I decided that the sanest thing is to unconditionally disable messing with the fpu control word.

In fact, for x86_64* this is unnecessary (it uses sse for doubles) and for i686* this will fall back to `-ffloat-store` (or else one can add `-msse` to `CFLAGS`, then the configure will use sse for doubles and there won't be a need for float-store.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [WIP] sympow: on musl, do not mess with fpu control word
  2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
                   ` (2 preceding siblings ...)
  2021-11-23  2:31 ` tornaria
@ 2021-11-23 12:00 ` tornaria
  2021-11-25  2:57 ` [PR PATCH] [Updated] " tornaria
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: tornaria @ 2021-11-23 12:00 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

New comment by tornaria on void-packages repository

https://github.com/void-linux/void-packages/pull/34209#issuecomment-976444205

Comment:
Please don't merge yet. There is another issue on sympow (on i686) that merits looking into, as evidenced by
```
$ sympow -curve [1,-1,1,1,-1] -moddeg
sympow 2.023.6 RELEASE  (c) Mark Watkins --- see README and COPYING for details
Minimal model of curve  is [1,-1,1,1,-1]
At 2: Inertia Group is  C1 MULTIPLICATIVE REDUCTION
**ERROR** Assertion failed
```

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PR PATCH] [Updated] [WIP] sympow: on musl, do not mess with fpu control word
  2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
                   ` (3 preceding siblings ...)
  2021-11-23 12:00 ` [WIP] " tornaria
@ 2021-11-25  2:57 ` tornaria
  2021-11-25  3:01 ` tornaria
  2021-11-25 12:33 ` [PR PATCH] [Merged]: " leahneukirchen
  6 siblings, 0 replies; 8+ messages in thread
From: tornaria @ 2021-11-25  2:57 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

There is an updated pull request by tornaria against master on the void-packages repository

https://github.com/tornaria/void-packages sympow
https://github.com/void-linux/void-packages/pull/34209

[WIP] sympow: on musl, do not mess with fpu control word
It turns out doing that results in broken precision, and not doing it
fixes it.

A simple way to test it is to run

    $ sympow -curve '[0,0,0,0,1]' -analrank
    ...
    Analytic Rank is 0 : L-value 7.01091e-01

When broken this prints "7.01092e-01" instead.

The actual value computed to 128 bits with pari is:

    $ echo 'elllseries(ellinit([0,0,0,0,1]),1)' | gp -q
    0.70109105266272713058750953952514706773

so the glibc output is the correct one.

It seems to me that what is broken is not computing but printing, but
this is important since sagemath parses sympow output.

This affects sagemath doctests as in

    $ sage -t src/sage/lfunctions/sympow.py
    ...
    (3 failures)

After this commit, the doctest in question passes.

<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **YES**

The corresponding doctest in sagemath passes on musl with this change; it was failing before.

A patch file from https://github.com/void-linux/void-packages/pull/34209.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-sympow-34209.patch --]
[-- Type: text/x-diff, Size: 7598 bytes --]

From 5178b7b871a83c18bf4196e8be67f061ed2f835e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gonzalo=20Tornar=C3=ADa?= <tornaria@cmat.edu.uy>
Date: Mon, 22 Nov 2021 19:24:50 -0300
Subject: [PATCH] sympow: do not mess with fpu control word if possible

Also: add basic tests to detect fpu miscompilation.

The program sets the fpu to 53 bit mode to get consistent results. It
turns out that doing this on musl breaks fmt_fp (-> printf) which uses
long double to format floats. Note this is an issue even on 64 bit musl,
although setting fpu to 53 bits is unnecessary on 64 bits because of the
default -mfpmath=sse.

The way Configure works: it tries different combinations of flags which
exercise different methods to change the fpu control word until one
works, meaning doubles are effectively 53 bits.  The fix here is to try
first NOT touching the fpu control word. On x86_64 using the default
-mfpmath=sse this will succeed bypassing all fpucw modification.

For i686 using -mfpmath=387 (always if sse2 not available) the code will
fall back to changing the fpu control word as before. Since this is not
a problem for glibc, everything still works. I expect i686-musl to still
be broken, but we don't support that arch.

A simple way to test the bug on musl is to run

    $ sympow -curve '[0,0,0,0,1]' -analrank
    ...
    Analytic Rank is 0 : L-value 7.01091e-01

When the fpucw is changed, on musl this prints "7.01092e-01" instead.

The actual value computed to 128 bits with pari is:

    $ echo 'elllseries(ellinit([0,0,0,0,1]),1)' | gp -q
    0.70109105266272713058750953952514706773

so the glibc output is the correct one.

Again: what is broken is not computing but printing, as fmt_fp in musl
uses long double, which means messing with fpu control word breaks it.
This is important since sagemath parses sympow output.

This affects sagemath doctests as in

    $ sage -t src/sage/lfunctions/sympow.py
    ...
    (3 failures)

After this commit, the doctest in question passes on the 3 supported
archs.
---
 srcpkgs/sympow/files/test                     | 51 ++++++++++++++++++
 srcpkgs/sympow/files/test.out                 | 54 +++++++++++++++++++
 .../first-try-without-messing-the-fpucw.patch | 14 +++++
 srcpkgs/sympow/template                       | 10 +++-
 4 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 srcpkgs/sympow/files/test
 create mode 100644 srcpkgs/sympow/files/test.out
 create mode 100644 srcpkgs/sympow/patches/first-try-without-messing-the-fpucw.patch

diff --git a/srcpkgs/sympow/files/test b/srcpkgs/sympow/files/test
new file mode 100644
index 000000000000..5892937085f2
--- /dev/null
+++ b/srcpkgs/sympow/files/test
@@ -0,0 +1,51 @@
+#! /bin/sh
+
+# Quick minimal test for sympow
+#
+# Compute modular degree and analytic rank for some curves so we check
+# that it works and also that fpu arithmetic and output are ok
+
+# setup sympow to run from cwd
+export SYMPOW_PKGDATADIR=.
+export SYMPOW_PKGLIBDIR=.
+mkdir -p sympow_cache
+export SYMPOW_CACHEDIR=sympow_cache
+
+E11a=[0,-1,1,-10,-20]
+E37a=[0,0,1,-1,0]
+E389a=[0,1,1,-2,0]
+E5077a=[0,0,1,-7,6]
+E4=[1,-1,0,-79,289]
+E5=[0,0,1,-79,342]
+E6=[1,1,0,-2582,48720]
+E7=[0,0,0,-10012,346900]
+
+echo "### Compute modular degree for some curves"
+echo "# 11a"
+./sympow -quiet -curve $E11a -moddeg
+echo "# 37a"
+./sympow -quiet -curve $E37a -moddeg
+echo "# 389a"
+./sympow -quiet -curve $E389a -moddeg
+echo "# 5077a"
+./sympow -quiet -curve $E5077a -moddeg
+echo "# 234446a"
+./sympow -quiet -curve $E4 -moddeg
+
+echo "### Compute analytic rank for some curves"
+echo "# 11a"
+./sympow -quiet -curve $E11a -analrank
+echo "# 37a"
+./sympow -quiet -curve $E37a -analrank
+echo "# 389a"
+./sympow -quiet -curve $E389a -analrank
+echo "# 5077a"
+./sympow -quiet -curve $E5077a -analrank
+echo "# 234446a"
+./sympow -quiet -curve $E4 -analrank
+echo "# curve of rank 5"
+./sympow -quiet -curve $E5 -analrank
+echo "# curve of rank 6"
+./sympow -quiet -curve $E6 -analrank
+echo "# curve of rank 7"
+./sympow -quiet -curve $E7 -analrank
diff --git a/srcpkgs/sympow/files/test.out b/srcpkgs/sympow/files/test.out
new file mode 100644
index 000000000000..b95c57494135
--- /dev/null
+++ b/srcpkgs/sympow/files/test.out
@@ -0,0 +1,54 @@
+### Compute modular degree for some curves
+# 11a
+Minimal model of curve  is [0,-1,1,-10,-20]
+Maximal number of terms is 14
+Modular Degree is 1
+# 37a
+Minimal model of curve  is [0,0,1,-1,0]
+Maximal number of terms is 48
+Modular Degree is 2
+# 389a
+Minimal model of curve  is [0,1,1,-2,0]
+Maximal number of terms is 512
+Modular Degree is 40
+# 5077a
+Minimal model of curve  is [0,0,1,-7,6]
+Maximal number of terms is 8130
+Modular Degree is 1984
+# 234446a
+Minimal model of curve  is [1,-1,0,-79,289]
+Maximal number of terms is 518272
+Modular Degree is 334976
+### Compute analytic rank for some curves
+# 11a
+Minimal model of curve  is [0,-1,1,-10,-20]
+Maximal number of terms is 7
+Analytic Rank is 0 : L-value 2.53842e-01
+# 37a
+Minimal model of curve  is [0,0,1,-1,0]
+Maximal number of terms is 10
+Analytic Rank is 1 : L'-value 3.06000e-01
+# 389a
+Minimal model of curve  is [0,1,1,-2,0]
+Maximal number of terms is 43
+Analytic Rank is 2 : leading L-term 7.59317e-01
+# 5077a
+Minimal model of curve  is [0,0,1,-7,6]
+Maximal number of terms is 128
+Analytic Rank is 3 : leading L-term 1.73185e+00
+# 234446a
+Minimal model of curve  is [1,-1,0,-79,289]
+Maximal number of terms is 1064
+Analytic Rank is 4 : leading L-term 8.94385e+00
+# curve of rank 5
+Minimal model of curve  is [0,0,1,-79,342]
+Maximal number of terms is 7858
+Analytic Rank is 5 : leading L-term 3.02857e+01
+# curve of rank 6
+Minimal model of curve  is [1,1,0,-2582,48720]
+Maximal number of terms is 184767
+Analytic Rank is 6 : leading L-term 3.20781e+02
+# curve of rank 7
+Minimal model of curve  is [0,0,0,-10012,346900]
+Maximal number of terms is 1324544
+Analytic Rank is 7 : leading L-term 1.32517e+03
diff --git a/srcpkgs/sympow/patches/first-try-without-messing-the-fpucw.patch b/srcpkgs/sympow/patches/first-try-without-messing-the-fpucw.patch
new file mode 100644
index 000000000000..a7b17819b7e5
--- /dev/null
+++ b/srcpkgs/sympow/patches/first-try-without-messing-the-fpucw.patch
@@ -0,0 +1,14 @@
+--- a/Configure	2020-04-16 14:12:56.000000000 -0300
++++ b/Configure	2021-11-24 16:04:43.647041822 -0300
+@@ -154,10 +154,10 @@
+     try_add_CFLAG $FLAG bypass && break
+ done
+ 
+ # Select the most appropriate FPU control scheme
+-for FLAG in '-DISOC99_FENV' '-DFPUCONTROLH' '-Dx86'; do
++for FLAG in '' '-DISOC99_FENV' '-DFPUCONTROLH' '-Dx86'; do
+     try_add_CFLAG $FLAG && break
+ done
+ 
+ # Some flags to try as last resort.  These hurt performance, so only add
+ # them if needed.
diff --git a/srcpkgs/sympow/template b/srcpkgs/sympow/template
index d484cf847f57..b81d2d4097c8 100644
--- a/srcpkgs/sympow/template
+++ b/srcpkgs/sympow/template
@@ -1,7 +1,7 @@
 # Template file for 'sympow'
 pkgname=sympow
 version=2.023.6
-revision=2
+revision=3
 wrksrc=${pkgname}-v${version}
 build_style=configure
 make_build_target=all
@@ -15,6 +15,8 @@ distfiles="https://gitlab.com/rezozer/forks/sympow/-/archive/v${version}/sympow-
 checksum=d153530dfdd46da05c954121640e50771064536fedc22c7fef24fb11083172ef
 nocross=yes # runs binaries built for target
 
+CFLAGS="-Wno-unused-result"
+
 do_configure() {
 	PREFIX=/usr ./Configure
 }
@@ -22,3 +24,9 @@ do_configure() {
 post_install() {
 	vlicense COPYING
 }
+
+do_check() {
+	echo "Testing sympow ..."
+	sh "${FILESDIR}"/test > test.log
+	diff "${FILESDIR}"/test.out test.log && echo PASS
+}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: sympow: on musl, do not mess with fpu control word
  2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
                   ` (4 preceding siblings ...)
  2021-11-25  2:57 ` [PR PATCH] [Updated] " tornaria
@ 2021-11-25  3:01 ` tornaria
  2021-11-25 12:33 ` [PR PATCH] [Merged]: " leahneukirchen
  6 siblings, 0 replies; 8+ messages in thread
From: tornaria @ 2021-11-25  3:01 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

New comment by tornaria on void-packages repository

https://github.com/void-linux/void-packages/pull/34209#issuecomment-978776823

Comment:
This is ready now. I added a few simple tests which should catch miscompilation and precision issues (it does catch the original issue on x86_64-musl, and it also catches the fault on i686 introduced by the first attempt at solution).

I explained the issue and the solution with detail in the commit message.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PR PATCH] [Merged]: sympow: on musl, do not mess with fpu control word
  2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
                   ` (5 preceding siblings ...)
  2021-11-25  3:01 ` tornaria
@ 2021-11-25 12:33 ` leahneukirchen
  6 siblings, 0 replies; 8+ messages in thread
From: leahneukirchen @ 2021-11-25 12:33 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

There's a merged pull request on the void-packages repository

sympow: on musl, do not mess with fpu control word
https://github.com/void-linux/void-packages/pull/34209

Description:
It turns out doing that results in broken precision, and not doing it
fixes it.

A simple way to test it is to run

    $ sympow -curve '[0,0,0,0,1]' -analrank
    ...
    Analytic Rank is 0 : L-value 7.01091e-01

When broken this prints "7.01092e-01" instead.

The actual value computed to 128 bits with pari is:

    $ echo 'elllseries(ellinit([0,0,0,0,1]),1)' | gp -q
    0.70109105266272713058750953952514706773

so the glibc output is the correct one.

It seems to me that what is broken is not computing but printing, but
this is important since sagemath parses sympow output.

This affects sagemath doctests as in

    $ sage -t src/sage/lfunctions/sympow.py
    ...
    (3 failures)

After this commit, the doctest in question passes.

<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **YES**

The corresponding doctest in sagemath passes on musl with this change; it was failing before.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-11-25 12:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 23:43 [PR PATCH] sympow: on musl, do not mess with fpu control word tornaria
2021-11-22 23:50 ` q66
2021-11-23  2:28 ` [PR PATCH] [Updated] " tornaria
2021-11-23  2:31 ` tornaria
2021-11-23 12:00 ` [WIP] " tornaria
2021-11-25  2:57 ` [PR PATCH] [Updated] " tornaria
2021-11-25  3:01 ` tornaria
2021-11-25 12:33 ` [PR PATCH] [Merged]: " leahneukirchen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).