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

Upgrade eclib to version 20100711 #9476

Closed
JohnCremona opened this issue Jul 11, 2010 · 17 comments
Closed

Upgrade eclib to version 20100711 #9476

JohnCremona opened this issue Jul 11, 2010 · 17 comments

Comments

@JohnCremona
Copy link
Member

I have made several enhancements to eclib:

  1. Support for minus space modular symbols
  2. Some sparse linear algebra improvements

The new version is called eclib-20100711 since it is more than just a patch-level change. See below for a link to the spkg.

The interface in sage/libs/cremona has been updated accordingly in the patch; this depends on #9441

CC: @williamstein @categorie

Component: packages: standard

Keywords: eclib modular symbols

Author: John Cremona

Reviewer: Robert Miller

Merged: sage-4.5.3.alpha0

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

@JohnCremona
Copy link
Member Author

comment:1

The new spkg is here: http://www.warwick.ac.uk/staff/J.E.Cremona/eclib-20100711.spkg

@JohnCremona
Copy link
Member Author

Attachment: trac_9476-eclib.patch.gz

Applies after eclib-20100711.patch and trac_9441-atkin-lehner.patch

@JohnCremona

This comment has been minimized.

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jul 17, 2010

comment:3

I'm reviewing this and #9441 at the same time. So far it compiles just fine with sage-4.5 final, on Intel OS X 10.6.4, and I'm currently running tests. I'll also give it a try on geom.math, which has begun at the moment.

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jul 17, 2010

comment:4

Looks good on OS X. Same on geom.math.

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jul 17, 2010

Reviewer: Robert Miller

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jul 17, 2010

comment:5

Small reviewer patch coming up in a minute!

@rlmill rlmill mannequin added s: needs work and removed s: positive review labels Jul 17, 2010
@JohnCremona
Copy link
Member Author

comment:6

Thanks!

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jul 17, 2010

comment:7

Hmm. I think this might have something to do with one of the things I saw on #9247.

I'm attaching the reviewer patch, which causes the following:

sage -t  "devel/sage-main/sage/schemes/elliptic_curves/ell_modular_symbols.py"
**********************************************************************
File "/Users/rlmill/sage-4.5.eclib-test/devel/sage-main/sage/schemes/elliptic_curves/ell_modular_symbols.py", line 429:
    sage: M=sage.schemes.elliptic_curves.ell_modular_symbols.ModularSymbolECLIB(E,-1)
Expected nothing
Got:
    Warning : Could not normalize the modular symbols, maybe all further results will be multiplied by -1, 2 or -2.
**********************************************************************
File "/Users/rlmill/sage-4.5.eclib-test/devel/sage-main/sage/schemes/elliptic_curves/ell_modular_symbols.py", line 438:
    sage: M=sage.schemes.elliptic_curves.ell_modular_symbols.ModularSymbolECLIB(E,-1)
Expected nothing
Got:
    Warning : Could not normalize the modular symbols, maybe all further results will be multiplied by -1, 2 or -2.
**********************************************************************

John,

Can you give some info about what's going on here?

@rlmill rlmill mannequin added s: needs info and removed s: needs work labels Jul 17, 2010
@JohnCremona
Copy link
Member Author

comment:8

I have added Chris W to the CC list since we'll need his input, as he wrote ell_modular_symbols.

I agree that that file needs updating as a consequence of my upgrade; but that can be done on a separate ticket?

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jul 17, 2010

comment:9

John,

I thought that eclib was propagating that warning, but clearly it's coming from ell_modular_symbols.py. Have a look at the new ref patch, and let me know what you think.

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jul 17, 2010

comment:10

Attachment: trac_9476-remove-not-implemented-error.patch.gz

@rlmill rlmill mannequin added s: needs review and removed s: needs info labels Jul 17, 2010
@williamstein
Copy link
Contributor

comment:11

reviewer addendum looks good to me.

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 7, 2010

comment:12

I'm having difficulty getting the new package:

$ wget http://www.warwick.ac.uk/staff/J.E.Cremona/eclib-20100711.spkg
--19:50:47--  http://www.warwick.ac.uk/staff/J.E.Cremona/eclib-20100711.spkg
           => `eclib-20100711.spkg'
Resolving www.warwick.ac.uk... 137.205.243.107
Connecting to www.warwick.ac.uk|137.205.243.107|:80... connected.
HTTP request sent, awaiting response... 

Can someone check its availability and perhaps put a copy on the Sage cluster?

Also, should I apply both patches, too?

@qed777 qed777 mannequin added s: needs info and removed s: positive review labels Aug 7, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 8, 2010

comment:14

The package is available now. I'll include both patches.

@qed777 qed777 mannequin added s: needs review and removed s: needs info labels Aug 8, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 9, 2010

Merged: sage-4.5.3.alpha0

@qed777 qed777 mannequin removed the s: positive review label Aug 9, 2010
@qed777 qed777 mannequin closed this as completed Aug 9, 2010
@JohnCremona
Copy link
Member Author

comment:17

Sorry not to have responded earlier but I was on holiday for a few days. I think they were doing some network updating at U of Warwick, which could explain why you could not get the file. Glad it's fixed -- and thanks for the review.

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

2 participants