Github messages for voidlinux
 help / color / mirror / Atom feed
From: tornaria <tornaria@users.noreply.github.com>
To: ml@inbox.vuxu.org
Subject: Re: [PR PATCH] [Updated] sympow: on musl, do not mess with fpu control word
Date: Tue, 23 Nov 2021 03:28:20 +0100	[thread overview]
Message-ID: <20211123022820.tVMYxsElw_xSMW_ItiGbYkV9mJFcgOiZ9BVfyla9HB8@z> (raw)
In-Reply-To: <gh-mailinglist-notifications-41a7ca26-5023-4802-975b-f1789d68868e-void-packages-34209@inbox.vuxu.org>

[-- 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

  parent reply	other threads:[~2021-11-23  2:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-22 23:43 [PR PATCH] " tornaria
2021-11-22 23:50 ` q66
2021-11-23  2:28 ` tornaria [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211123022820.tVMYxsElw_xSMW_ItiGbYkV9mJFcgOiZ9BVfyla9HB8@z \
    --to=tornaria@users.noreply.github.com \
    --cc=ml@inbox.vuxu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).