Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#6955 closed defect (fixed)

update simon denis pari-scripts

Reported by: wuthrich Owned by:
Priority: major Milestone: sage-4.4
Component: elliptic curves Keywords: two descent,
Cc: Merged in: sage-4.4.alpha0
Authors: John Cremona Reviewers: Chris Wuthrich
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

I found out that sage comes with old versions of the files ell.gp, ellQ.gp, qfsolve.gp. This should be updated. The newest version can be found at http://www.math.unicaen.fr/~simon/ .

Attachments (6)

ell.gp (69.4 KB) - added by cremona 12 years ago.
ellQ.gp (49.8 KB) - added by cremona 12 years ago.
qfsolve.gp (31.1 KB) - added by cremona 12 years ago.
trac_6955-simon-update.patch (6.4 KB) - added by cremona 12 years ago.
Applies to 4.3.5
trac_6955-simon-update-extcode.patch (59.6 KB) - added by cremona 12 years ago.
Apply in SAGE_ROOT/data/extcode before other patch
trac_6955-simon-update_reviewer.patch (6.3 KB) - added by wuthrich 12 years ago.
replaces the previous patch of this name

Download all attachments as: .zip

Change History (24)

comment:1 Changed 12 years ago by wuthrich

This should be very easy to do, but I don't know how to submit a patch for this. Sorry.

comment:2 Changed 12 years ago by davidloeffler

  • Owner changed from davidloeffler to (none)

comment:3 Changed 12 years ago by cremona

  • Report Upstream set to N/A

See also #5153. We'll need to check that the version on Denis's home page is at least as up-to-date as the one he sent me which apparently fixed the bug I sent him. I'll also try to find that email correspondence and add it to one of these tickets.

comment:4 Changed 12 years ago by cremona

I have updated the scripts (attached to the ticket) to the ones on DS's home page on 3 April 2010, which are dated as follows:

  • ell.gp (v. 2009-03-25)
  • ellQ.gp (v. 2008-04-29)
  • qfsolve.gp (v. 2008-02-26)

The patch (to appear) applies to 4.3.5 and fixes some small things: Simon's variable DEBUGLEVEL has been renamed DEBUGLEVEL_ell, and some of the verbose output changes a little. ALso, some "generators" appear in a different order or are otherwise (mathematically) trivially different.

This does not fix other issues with these scripts, as in #5153.

Changed 12 years ago by cremona

Changed 12 years ago by cremona

Changed 12 years ago by cremona

Changed 12 years ago by cremona

Applies to 4.3.5

comment:5 Changed 12 years ago by cremona

  • Authors set to John Cremona
  • Status changed from new to needs_review

The three scripts should replace those in SAGE_ROOT/data/extcode/pari/simon/

Changed 12 years ago by cremona

Apply in SAGE_ROOT/data/extcode before other patch

comment:6 Changed 12 years ago by cremona

The patch trac_6955-simon-update-extcode.patch should be applied in the directory SAGE_ROOT/data/extcode in addition to the usual patch for sage-main. The gp files on the ticket are just for reference.

comment:7 follow-up: Changed 12 years ago by wuthrich

Sorry, John, I am a newbie to anything that is outside the devel-tree. Can you tell me exactly what I have to do (if you want me to review it) ?

Chris.

comment:8 in reply to: ↑ 7 Changed 12 years ago by cremona

Replying to wuthrich:

Sorry, John, I am a newbie to anything that is outside the devel-tree.

So was I before I did this!

Can you tell me exactly what I have to do (if you want me to review it) ?

OK. You need to be careful, since you will be changing files outside $SAGE_ROOT/devel, which are therefore not covered by the cloning system, so you have to work a bit harder both to apply the patches and to unapply them. I will assume that you will use mercurial queues (but it would be possible without).

First make a clone in the usual way, say called simon, so you have created $SAGE_ROOT/devel/sage-simon as a copy of $SAGE_ROOT/devel/sage-main.

Now apply the patch to the extcode:

cd SAGE_ROOT/data/extcode  
hg qseries      # test that queues have been initialised here;  if not, do hg qinit first
hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6955/trac_6955-simon-update-extcode.patch
hg qpush

Now apply the ordinary patch:

cd ../../devel/sage-simon
hg qinit  # if not already done
hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6955/trac_6955-simon-update.patch
hg qpush
sage -b

That's it applied. Run, test, as much as you like. To reverse the changes:

# in devel/sage-simon
hg qpop
cd ../../data/extcode
hg qpop
sage -b

Chris.

comment:9 Changed 12 years ago by wuthrich

  • Reviewers set to Chris Wuthrich

Thanks a lot for the very helpful comments. I assume the next time I would be able to make a patch myself for this.

All tests passed after having deleted the first change in the patch (it added a space at the start of the file.)

The modified patch corrects this.

Changed 12 years ago by wuthrich

replaces the previous patch of this name

comment:10 Changed 12 years ago by wuthrich

  • Status changed from needs_review to positive_review

So I give a positive review. The two patches trac_6955-simon-update_reviewer.patch and trac_6955-simon-update-extcode.patch have to be applied as described by John.

comment:11 Changed 12 years ago by cremona

Thanks -- sorry about that space, which I thought I had put in before making the patch and not after! It has been bugging me ever since.

comment:12 follow-up: Changed 12 years ago by jhpalmieri

  • Status changed from positive_review to needs_work

The reviewer patch doesn't apply cleanly. Is it okay to just delete the portion of the patch for the file sage/schemes/elliptic_curves/gp_simon.py?

comment:13 in reply to: ↑ 12 ; follow-up: Changed 12 years ago by cremona

Replying to jhpalmieri:

The reviewer patch doesn't apply cleanly. Is it okay to just delete the portion of the patch for the file sage/schemes/elliptic_curves/gp_simon.py?

No, that part is crucial. Did you see that the review patch is instead of my patch called trac_6955-simon-update.patch, and not a second one to b applied after it? The only difference between them is that my patch wrongly inserts a space in the first line of the file ell_number_field.py, and Chris's patch removes that bit of my patch,

comment:14 in reply to: ↑ 13 Changed 12 years ago by jhpalmieri

Replying to cremona:

Did you see that the review patch is instead of my patch called trac_6955-simon-update.patch, and not a second one to b applied after it?

Ah, no, I'd missed that. Sorry.

comment:15 Changed 12 years ago by jhpalmieri

  • Status changed from needs_work to needs_review

comment:16 Changed 12 years ago by jhpalmieri

  • Status changed from needs_review to positive_review

comment:17 Changed 12 years ago by jhpalmieri

  • Merged in set to sage-4.4.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

Merged in 4.4.alpha0:

  • trac_6955-simon-update-extcode.patch
  • trac_6955-simon-update_reviewer.patch

comment:18 Changed 12 years ago by cremona

Many thanks, jhp -- seems that you are release manager, and busy!

Note: See TracTickets for help on using tickets.