Skip to content

Comments

sage: adapt eclib test expected output, revert cypari2 update#114747

Merged
SuperSandro2000 merged 2 commits intoNixOS:masterfrom
collares:eclib-update
Mar 7, 2021
Merged

sage: adapt eclib test expected output, revert cypari2 update#114747
SuperSandro2000 merged 2 commits intoNixOS:masterfrom
collares:eclib-update

Conversation

@collares
Copy link
Member

@collares collares commented Mar 1, 2021

Motivation for this change

eclib was updated in #114425, but the test output changed slightly. Upstream is still on eclib 20190909, so I cannot just import an upstream patch.

Should unbreak sage. I've verified that the test outputs are the same up to minor formatting changes.

@ofborg ofborg bot requested review from 7c6f434c, omasanori and timokau March 1, 2021 13:06
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 1, 2021
@collares collares changed the title eclib: 20210223 -> 20210226 eclib: 20210223 -> 20210226 (and fix sage test expectations) Mar 1, 2021
@collares
Copy link
Member Author

collares commented Mar 1, 2021

@ofborg build sage

@collares collares marked this pull request as draft March 1, 2021 14:05
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Mar 1, 2021
@collares collares changed the title eclib: 20210223 -> 20210226 (and fix sage test expectations) sage: adapt eclib test expected output due to update Mar 1, 2021
@collares
Copy link
Member Author

collares commented Mar 1, 2021

@omasanori @timokau @7c6f434c This is now ready for review. Note that I initially tried to upgrade to eclib 20210226, but this new version causes extra test failures so we must be careful to not let other people update it for the time being. I reported this at JohnCremona/eclib#64, and the conclusion is that Sage requires a patch to set precision parameters properly under eclib 20210226.

The eclib 20210226 extra failures are:

sage -t --long --random-seed=0 /nix/store/k9x5qwik8cdr9vm782v5ha116j097qn4-sage-src-9.2/src/sage/schemes/elliptic_curves/ell_rational_field.py
**********************************************************************
File "/nix/store/k9x5qwik8cdr9vm782v5ha116j097qn4-sage-src-9.2/src/sage/schemes/elliptic_curves/ell_rational_field.py", line 1877, in sage.schemes.elliptic_curves.ell_rational_field.EllipticCurve_rational_field.?
Failed example:
    r, s, G = E.simon_two_descent(); r,s
Expected:
    (8, 8)
Got:
    After 10 attempts at enlargement, giving up!
    --points not proved 5-saturated,
    5-saturation failed!
    Failed to saturate MW basis at primes [ 5 ]
    5 After 10 attempts at enlargement, giving up!
    --points not proved 5-saturated,
    5-saturation failed!
    Failed to saturate MW basis at primes [ 5 ]
    5 After 10 attempts at enlargement, giving up!
    --points not proved 5-saturated,
    5-saturation failed!
    Failed to saturate MW basis at primes [ 5 ]
    (8, 8)
**********************************************************************
File "/nix/store/k9x5qwik8cdr9vm782v5ha116j097qn4-sage-src-9.2/src/sage/schemes/elliptic_curves/ell_rational_field.py", line 1883, in sage.schemes.elliptic_curves.ell_rational_field.EllipticCurve_rational_field.?
Failed example:
    E.simon_two_descent()
Expected:
    (2, 3, [(-1/4 : 2377/8 : 1), (323/4 : 1891/8 : 1)])
Got:
    5 (2, 3, [(-1/4 : 2377/8 : 1), (323/4 : 1891/8 : 1)])
**********************************************************************
1 item had failures:
   2 of  88 in sage.schemes.elliptic_curves.ell_rational_field.EllipticCurve_rational_field.?
    [849 tests, 2 failures, 195.74 s]

@collares collares marked this pull request as ready for review March 1, 2021 15:53
@dimpase
Copy link

dimpase commented Mar 1, 2021

@JohnCremona

@dimpase
Copy link

dimpase commented Mar 1, 2021

not sure what's prevening Sage's upgrade of eclib, apart from labour shortages.

@JohnCremona
Copy link

See https://trac.sagemath.org/ticket/31443

collares added 2 commits March 6, 2021 12:28
This reverts commit 7a3db26.

Updating this requires fixing Sage tests, which will be done in a
separate PR.
@collares collares requested review from FRidh and jonringer as code owners March 6, 2021 15:29
@collares collares changed the title sage: adapt eclib test expected output due to update sage: adapt eclib test expected output, revert cypari2 update Mar 6, 2021
@collares
Copy link
Member Author

collares commented Mar 6, 2021

I have added a separate commit reverting the cypari2 update that just landed on master, because it is causing test failures on aarch64. Upgrading cypari2 properly will be done on staging now that #115200 has been merged.

@ofborg ofborg bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Mar 6, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Mar 6, 2021
@SuperSandro2000 SuperSandro2000 merged commit 7e8cfa2 into NixOS:master Mar 7, 2021
@collares
Copy link
Member Author

collares commented Mar 7, 2021

@SuperSandro2000 Thanks for merging! It's much appreciated.

@collares collares deleted the eclib-update branch December 20, 2021 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants