-
-
Notifications
You must be signed in to change notification settings - Fork 514
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reject broken system PARI 2.11.3 and re-enable PARI library test in spkg-configure #29445
Comments
comment:1
The following tests currently fail (depending on the PARI version):
There are some other tests that seem PARI related, e.g.
and
Those tests could be repaired by just checking that the numerical noise is reasonable. |
This comment has been minimized.
This comment has been minimized.
comment:3
I have upgraded to pari 2.11.3 and that reproduces the problems. So I confirm that this is the problem. |
comment:4
Something seems to be messed up when installing PARI 2.11.3
But the expected output is |
Changed author from Jonathan Kliem to none |
This comment has been minimized.
This comment has been minimized.
comment:5
Maybe cypari 2.1.1 just cannot deal with pari 2.11.3, but maybe there is also a bug in pari 2.11.3. |
comment:6
Before I forget, this also looks serious
I don't think this element has norm |
comment:7
pari 2.11.3 is broken, see #29313. The test in the previous comment is ok though, both elements have norm 7. |
comment:8
Ok, I'm stupid. I redifined So what do we do. Reject pari 2.11.3 from system until this is fixed? Replying to @antonio-rojas:
|
comment:9
Replying to @kliem:
Yes, IMO that's what we should do - unpatched 2.11.3 gives incorrect answers which is not acceptable. |
comment:12
Devil's advocate: we shouldn't be in the business of detecting system versions down to the backported-commit-patch level. Distros should be in charge of not shipping broken software to their users, who probably don't want to be using a PARI that gives the wrong answer outside of Sage, either. If people hit this, they should complain to their distro to either backport the patch or mask the busted version. |
comment:13
spkg-configure is exactly this business, by definition But it shouldn't detect versions. It should detect absence of known bugs. |
comment:14
As far as I understand, the patch in the branch #29313 also includes a small doctest. Can we include this in spkg-configure? This is probably wrong syntax, but something like this might work:
|
comment:15
On the broken pari the output of this is |
comment:16
yes, that's the right way to fix it. Careful with the brackets - they are m4 quoting characters |
comment:17
Can you are someone put it in the right place? I can try to, but I'm really guessing here. |
comment:18
OK, will do |
Commit: |
Author: Jonathan Kliem, Matthias Koeppe |
comment:24
@mkoeppe: Thanks. Should I start a github workflow? Are you already doing it? |
comment:25
Please do. #29417 has the latest version |
comment:26
What about the original aim of this ticket (make tests pass regardless of the system pari version)? When using a patched 2.11.3 you'd still get many doctest failures (but correct nevertheless) |
comment:27
Should probably be taken care of in the same ticket |
comment:28
I would wait and see. I'm still willing to change the doctests such that both pari versions work, but I guess on a new ticket once this problem occurs. (I also wasn't sure how many of those things would disappear once this is patched, e.g. the +/- failures.) |
comment:29
You can see the exact failures in the patch for #29313 (but that patch only changes them to the 2.11.3 answer, so they would fail with <2.11.3) |
comment:30
Ok, seems to work https://github.com/kliem/sage-test-27122/actions Specifically for debian bulleye standard it detects the bug and doesn't use system pari (and those failures are gone). |
Reviewer: Jonathan Kliem |
Changed reviewer from Jonathan Kliem to Matthias Koeppe, Jonathan Kliem |
Changed branch from u/mkoeppe/fix_failing_doctests_because_of_representation_depending_on_pari_version to |
Changed commit from |
comment:34
it is a buggy test
Don't mix up See #29554 for a fix |
There are several failing doctests depending on the PARI version of the system, e.g.
see e.g.
https://github.com/mkoeppe/sage/runs/542655863
This is the case because debian bullseye uses pari 2.11.3 (instead of pari 2.11.3). Many instances seem to fail, just because the choice of generators has changed. However, there is at least one thing really strange when upgrading to pari 2.11.3:
CC: @mkoeppe @jdemeyer @orlitzky @dimpase
Component: number fields
Keywords: PARI
Author: Jonathan Kliem, Matthias Koeppe
Branch:
c4a0a22
Reviewer: Matthias Koeppe, Jonathan Kliem
Issue created by migration from https://trac.sagemath.org/ticket/29445
The text was updated successfully, but these errors were encountered: