search

sagemath / sage

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

Upgrade to cypari2-2.0.3 #26442

Closed jdemeyer closed 5 years ago

jdemeyer commented 6 years ago

Tarball: https://files.pythonhosted.org/packages/64/bd/93fd5d8e7515a87be0f74958d0e1dd1b67ac0d009c0ca4d36be891e72c49/cypari2-2.0.3.tar.gz

Depends on #27060

CC: @videlec @defeo @kiwifb

Component: packages: standard

Author: Jeroen Demeyer

Branch/Commit: cd62d45

Reviewer: Vincent Delecroix, Timo Kaufmann

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

jdemeyer commented 5 years ago
comment:93

After 2 days(!) of debugging, I finally found out that the problem that I was seeing in [comment:85] was caused by a broken implementation of HNF computation. I added a new commit to fix that.

jdemeyer commented 5 years ago
comment:94

I added some code to cypari2 to better check for potential problems like this (and this is how I determined the problem). But then I found other issues in Sage, which are fixed by the dependency #27060.

So it remains to make a new cypari2 release and then update this ticket.

timokau commented 5 years ago
comment:95

Replying to @jdemeyer:

After 2 days(!) of debugging, I finally found out that the problem that I was seeing in [comment:85] was caused by a broken implementation of HNF computation. I added a new commit to fix that.

Thank you for your work!

Not sure if the new version will need additional sage changes, but when I run current master (cc1d51071358d9deeeff65ba83a04da9735d4abd) with the patch from this branch I get

Failed example:
    complex_plot(zeta, (-30,30), (-30,30))
Expected:
    Graphics object consisting of 1 graphics primitive
Got:
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
    ERROR: sig_on_count = 1 during remove_from_pari_stack()
[about a million more repetitions of this]

For a lot of tests.

Could you elaborate why the commit I pointed out cannot possibly be responsible for the issue I'm seeing?

jdemeyer commented 5 years ago
comment:96

Replying to @timokau:

with the patch from this branch

Are you very sure about that? Please double check that you're using the right branch from this ticket (note that it has 3 commits on top of 8.6.rc1, you need to apply them all).

timokau commented 5 years ago
comment:97

Here are a bunch of logs of the failure:

http://sprunge.us/BBxE3I http://sprunge.us/o0dKgP http://sprunge.us/TemVir http://sprunge.us/LzAYUz

timokau commented 5 years ago
comment:98

Replying to @jdemeyer:

Replying to @timokau:

with the patch from this branch

Are you very sure about that? Please double check that you're using the right branch from this ticket.

Not the latest version you pushed 4hrs ago. Sorry about that.

jdemeyer commented 5 years ago
comment:99

Just now I updated cypari2 master. So please test with that and with the Sage patches (3 commits) from the branch here.

I won't push any additional changes until you report back.

timokau commented 5 years ago
comment:100

Alright, testing the current version of this ticket (https://github.com/sagemath/sagetrac-mirror/compare/62fe3948ea8ca4c91e877290ed23529b6e6f79fe...a185f05263d4a0a58bf8a95fd49ba9d31df195c3) with the current cypari2 master (35e66b5ad58839b771c8acfd1fe7d7e03678cc88).

I won't push any additional changes until you report back.

That will be tomorrow though. Getting too late for another full ptestlong.

timokau commented 5 years ago
comment:101

While updating cypari2 to latest master I also noticed that it was set to plain 2.0.2. I'm pretty sure the last ptestlong was with master, but I must've reset it at some time. So maybe the last ptestlong wasn't with master after all.

Anyways, we'll know more when the next test finishes.

timokau commented 5 years ago
comment:102
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 5378.1 seconds
    cpu time: 18224.2 seconds
    cumulative wall time: 20930.0 seconds

🎉

jdemeyer commented 5 years ago

Description changed:

--- 
+++ 
@@ -1 +1 @@
-**Tarball**: https://files.pythonhosted.org/packages/b4/0d/817d280300e525a879b1d68ae31a4500f29b82ee4a603be5b0ff9572976e/cypari2-2.0.2.tar.gz
+**Tarball**: https://files.pythonhosted.org/packages/64/bd/93fd5d8e7515a87be0f74958d0e1dd1b67ac0d009c0ca4d36be891e72c49/cypari2-2.0.3.tar.gz
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from a185f05 to cd62d45

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

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

1894ffaClean up HNF computation using PARI
cd62d45Upgrade to cypari2 version 2.0.3
timokau commented 5 years ago
comment:106

The latest changes are only documentation and I've run ptestlong with yesterday's version 4 times now to be sure. Lets go ahead with this.

jdemeyer commented 5 years ago
comment:107

Note that the dependency #27060 still needs review.

jdemeyer commented 5 years ago

Changed reviewer from Vincent Delecroix to Vincent Delecroix, Timo Kaufmann

vbraun commented 5 years ago

Changed branch from u/jdemeyer/upgrade_to_cypari2_2_0_0 to cd62d45