Skip to content
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

Update eclib to latest upstream release #10993

Closed
JohnCremona opened this issue Mar 23, 2011 · 174 comments
Closed

Update eclib to latest upstream release #10993

JohnCremona opened this issue Mar 23, 2011 · 174 comments

Comments

@JohnCremona
Copy link
Member

eclib has changed a lot since the last upgrade, mostly in parts which do not affect Sage much, but there are exceptions, for example the handling of bounds in saturation for elliptic curves over Q (see #10840).

As well as various changes in the source code, the new distribution in the new spkg now uses autotools throughout which should make building on different systems easier.

After installing it (and before "sage -b") apply the patch also.

spkg: http://homepages.warwick.ac.uk/staff/J.E.Cremona/ftp/progs/eclib-20120428.spkg

Apply: attachment: trac_10993-eclib.patch

Depends on #11354

CC: @kiwifb @sagetrac-fschulze

Component: packages: standard

Author: John Cremona

Reviewer: Frithjof Schulze, Jeroen Demeyer, Volker Braun, Leif Leonhardy

Merged: sage-5.1.beta0

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

@JohnCremona

This comment has been minimized.

@JohnCremona

This comment has been minimized.

@JohnCremona JohnCremona changed the title Update eclib to 201103?? Update eclib to 20110323 May 18, 2011
@JohnCremona
Copy link
Member Author

comment:5

Working on this at Sage Days 35 (18 Dec 2011) -- will be ready for review soon.

@JohnCremona
Copy link
Member Author

comment:6

Ready for review. Thanks to Simon King and Paul Zimmermann for help adding the ldconfig line to the spkg-install script, which was necessary after Giovanni Mascellani mascellani@poisson.phc.unipi.it added the SONAME lines to my Makefiles.

@JohnCremona

This comment has been minimized.

@JohnCremona JohnCremona changed the title Update eclib to 20110323 Update eclib to 20111217 Dec 18, 2011
@sagetrac-fschulze

This comment has been minimized.

@sagetrac-fschulze
Copy link
Mannequin

sagetrac-fschulze mannequin commented Dec 19, 2011

comment:9

I tested with sage-4.8.alpha4. The patche applies fine,
the new spkg builds and passes it's own testsuit,
all doctests pass

@sagetrac-fschulze
Copy link
Mannequin

sagetrac-fschulze mannequin commented Dec 19, 2011

Reviewer: fschulze

@jdemeyer
Copy link

jdemeyer commented Jan 9, 2012

Work Issues: rebase

@jdemeyer
Copy link

jdemeyer commented Jan 9, 2012

comment:10

I'm afraid this needs to be rebased to #11354.

@jdemeyer
Copy link

jdemeyer commented Jan 9, 2012

Dependencies: #11354

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

comment:134

Replying to @nexttime:

You may also simply wrap getenv() with an additional (preferably const char*) default parameter, returned if the environment variable isn't set, just like Python's os.environ.get("VAR_NAME", default_value).

(And the additional parameter might itself default to either "" or NULL. But if you provide a default parameter for your wrapper function, you won't be able to just overload getenv(). So it would be best to add a getenv() function with an additional non-optional parameter, then calling getenv(NON_DEFINED_NAME) would just return NULL, while getenv(NON_DEFINED_NAME, DEFAULT_VALUE) would return DEFAULT_VALUE, "transparently".)

Like this:

#include <cstdlib>
#include <iostream>

using std::getenv;

const char *getenv(const char *var_name, const char *default_value)
{
  const char *val = getenv(var_name);

  return val ? val : default_value;
}

int main(void)
{
  std::cout << "getenv(\"FOO\"): " << getenv("FOO") << std::endl;
  std::cout.clear();
  std::cout << std::endl;
  std::cout << "getenv(\"FOO\", \"bar\"): " << getenv("FOO", "bar") << std::endl;
  return 0;
}

which yields

getenv("FOO"): 
getenv("FOO", "bar"): bar

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

comment:135

Replying to @JohnCremona:

Please don't wait for yet another upstream code fix before making a decision on this.  This function is not used by code currently wrapped in Sage...

Just waiting for tests (actually of the April 19th spkg) on Solaris to finish...

(They passed on Linux x86_64.)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

comment:136

The link in the description was still referencing the 20120419 spkg.

@nexttime

This comment has been minimized.

@JohnCremona
Copy link
Member Author

comment:137

Replying to @nexttime:

The link in the description was still referencing the 20120419 spkg.

That's funny, I'm sure I changed it.  The spkg works for me (builds and all long tests pass).

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

comment:138

Replying to @nexttime:

Just waiting for tests (actually of the April 19th spkg) on Solaris to finish...

Don't know whether that's a regression, but some tests take really long (as mentioned), at least on mark2:

$ time ./sage -t --long devel/sage/sage/interfaces/mwrank.py devel/sage/sage/libs/cremona/ devel/sage/sage/libs/mwrank/ devel/sage/sage/schemes/elliptic_curves/

sage -t --long "devel/sage/sage/interfaces/mwrank.py"       
         [25.8 s]
sage -t --long "devel/sage/sage/libs/cremona/all.py"        
         [1.2 s]
sage -t --long "devel/sage/sage/libs/cremona/newforms.pyx"  
         [22.4 s]
sage -t --long "devel/sage/sage/libs/cremona/__init__.py"   
         [1.2 s]
sage -t --long "devel/sage/sage/libs/cremona/defs.pxi"      
         [1.2 s]
sage -t --long "devel/sage/sage/libs/cremona/mat.pyx"       
         [21.5 s]
sage -t --long "devel/sage/sage/libs/cremona/constructor.py"
         [24.9 s]
sage -t --long "devel/sage/sage/libs/cremona/homspace.pyx"  
         [23.3 s]
sage -t --long "devel/sage/sage/libs/mwrank/all.py"         
         [21.1 s]
sage -t --long "devel/sage/sage/libs/mwrank/__init__.py"    
         [1.2 s]
sage -t --long "devel/sage/sage/libs/mwrank/interface.py"   
         [72.7 s]
sage -t --long "devel/sage/sage/libs/mwrank/mwrank.pyx"     
         [60.1 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/all.py"
         [25.0 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_local_data.py"
         [39.4 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py"
         [1581.5 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/monsky_washnitzer.py"
         [71.1 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/modular_parametrization.py"
         [17.4 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/__init__.py"
         [1.2 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_field.py"
         [26.3 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/padic_lseries.py"
         [164.0 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ec_database.py"
         [15.5 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_wp.py"
         [15.2 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_number_field.py"
         [164.4 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_finite_field.py"
         [149.4 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/weierstrass_morphism.py"
         [15.6 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/mod5family.py"
         [14.0 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/heegner.py"
         [370.1 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_point.py"
         [162.9 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_egros.py"
         [207.2 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_curve_isogeny.py"
         [818.7 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/cm.py"
         [49.8 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/kodaira_symbol.py"
         [13.9 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/padics.py"
         [109.7 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_tate_curve.py"
         [22.8 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/constructor.py"
         [205.1 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/lseries_ell.py"
         [34.1 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_generic.py"
         [118.1 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/period_lattice.py"
         [41.7 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/gal_reps.py"
         [269.3 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_modular_symbols.py"
         [125.5 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/sha_tate.py"
         [474.3 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/gp_simon.py"
         [19.5 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/BSD.py"
         [111.4 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/descent_two_isogeny.pyx"
         [24.1 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_torsion.py"
         [28.6 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/formal_group.py"
         [62.7 s]
sage -t --long "devel/sage/sage/schemes/elliptic_curves/ell_padic_field.py"
         [15.0 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 5862.3 seconds

real    97m45.421s
user    86m41.715s
sys     2m42.827s

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

comment:139

Replying to @nexttime:

Replying to @nexttime:

Just waiting for tests (actually of the April 19th spkg) on Solaris to finish...

Don't know whether that's a regression, but some tests take really long (as mentioned), at least on mark2 ...

beta13 + MPIR 2.4.0 + MPFR 3.1.0 with all upstream patches; all (relevant) packages compiled with GCC 4.7.0 with -O3. mark2 is a Sun Blade 2500, 2x UltraSPARC III @ 1280 MHz IIRC. (I have to say that Mariah's two GMP-ECM background processes apparently don't get suspended, so the sysload is around 3, with Sage's tests run serially.)

@JohnCremona
Copy link
Member Author

comment:140

None of the changes to eclib would make a noticeable difference in the timings of any Sage doctests.  Many other changes in this testing version of Sage do have such an effect.

Unless you can prove me wrong ( by giving the timings on exactly the same Sage version without the new eclib)  ...

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

comment:141

Replying to @JohnCremona:

None of the changes to eclib would make a noticeable difference in the timings of any Sage doctests.  Many other changes in this testing version of Sage do have such an effect.

Unless you can prove me wrong ( by giving the timings on exactly the same Sage version without the new eclib)  ...

Nope. I see a slight slowdown for ell_rational_field.py on my Linux netbook as well (user 8m30.440s vs. 8m44.760s; haven't compared others), but one is beta11 compiled with GCC 4.6.3, while the other, slower one with your new spkg is beta10 compiled with GCC 4.7.0 (same flags, both with MPIR 2.4.0). So also the compiler version might make a difference, or it's just "random noise".

Anyway, someone else would have to prove you wrong.., ;-)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

comment:142

Ok, latest spkg (20120421) looks clean, and also passes all relevant tests on Solaris SPARC (32-bit) as well as on Linux x86_64, so finally setting it to positive review, assuming Volker agrees.

(I guess John also prefers to [hopefully] get it into Sage 5.0 as is, rather than adding even more upstream changes which would again need review and testing.)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 21, 2012

Changed reviewer from Frithjof Schulze, Jeroen Demeyer to Frithjof Schulze, Jeroen Demeyer, Volker Braun, Leif Leonhardy

@vbraun
Copy link
Member

vbraun commented Apr 21, 2012

comment:143

Sounds great!

@JohnCremona
Copy link
Member Author

comment:144

Replying to @vbraun:

Sounds great!
Thanks to both!

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 22, 2012

comment:145

Just in case you're still listening, the new spkg also passes all [IMHO] relevant tests on Linux ppc64 (SLES 11, POWER7) with Sage 5.0.beta9 and #12829, GCC 4.6.3.

(And I haven't observed speed regressions; this time I've run the tests [in parallel] with the old and the new spkg with the same Sage installation, compiler and flags.)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 22, 2012

Changed work issues from ldconfig in spkg-install to none

@jdemeyer jdemeyer modified the milestones: sage-5.0, sage-5.1 Apr 22, 2012
@SnarkBoojum
Copy link
Mannequin

SnarkBoojum mannequin commented Apr 24, 2012

comment:148

I checked that eclib-20120421.spkg compiles well on my ARM system running ubuntu. The patch applied with fuzz, and "sage -ba-force" was a success too.

@JohnCremona
Copy link
Member Author

comment:149

Replying to @vbraun:

Sounds great!

@JohnCremona

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 29, 2012

comment:150

Yet another upstream update?

Note that the link target in the description is still .../eclib-20120421.spkg, perhaps just some copy-paste accident (cf. https://github.com/sagemath/sage/issues/10993?action=diff&version=149).

@JohnCremona
Copy link
Member Author

comment:151

It's a tiny bug fix in something not used in Sage, and as the target has moved to 5.1 I thought I would change the spkg.  Sorry if that causes more work in testing.

I'm finding it very hard to edit trac tickets since the version was changed, especially on a netbook while travelling.  I only see one place on the ticket where the spkg name is given, in the Description, and I changed that.  If there's another one somewhere (why would there be two?) and you can change it to match, please do.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 29, 2012

comment:152

Replying to @JohnCremona:

It's a tiny bug fix in something not used in Sage, and as the target has moved to 5.1 I thought I would change the spkg.  Sorry if that causes more work in testing.

I'm finding it very hard to edit trac tickets since the version was changed, especially on a netbook while travelling.  I only see one place on the ticket where the spkg name is given, in the Description, and I changed that.  If there's another one somewhere (why would there be two?) and you can change it to match, please do.

No, but in the description we currently again have a link target different to its name:

'''spkg''':  [http://homepages.warwick.ac.uk/staff/J.E.Cremona/ftp/progs/eclib-20120421.spkg //homepages.warwick.ac.uk/staff/J.E.Cremona/ftp/progs/eclib-20120428.spkg]

It's sufficient to just use

'''spkg''':  [http://homepages.warwick.ac.uk/staff/J.E.Cremona/ftp/progs/eclib-20120428.spkg]

(where the brackets are optional if you don't specify an [alternate] name for the link, but I always use them).

Made that change, although technically the new spkg would need new review.

@jdemeyer
Copy link

jdemeyer commented May 6, 2012

Merged: sage-5.1.beta0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants