Ticket #3632 (closed defect: fixed)

Opened 5 months ago

Last modified 5 months ago

[with patch,with positive review] small bug in p-adic heights

Reported by: wuthrich Assigned to: was
Priority: minor Milestone: sage-3.0.6
Component: number theory Keywords:
Cc:

Description 

sage: E = EllipticCurve([37,0])
sage: E.padic_regulator(5)

gives a Assertion Error.

The included patch corrects this.

Attachments

trac.3632.patch (0.8 kB) - added by wuthrich on 07/10/2008 06:11:54 AM.
trac.3632.extra1.patch (1.5 kB) - added by wuthrich on 07/11/2008 03:21:38 AM.
trac.3632.extra2.patch (2.4 kB) - added by dmharvey on 07/11/2008 06:10:02 AM.

Change History

07/10/2008 06:11:54 AM changed by wuthrich

  • attachment trac.3632.patch added.

07/10/2008 06:13:41 AM changed by mabshoff

  • milestone set to sage-3.0.5.

07/10/2008 04:27:48 PM changed by dmharvey

  • summary changed from [with patch, needs review] small bug in p-adic heights to [with patch, needs work] small bug in p-adic heights.

I hate to do this.... it's obviously a perfectly good patch.... but the documentation needs to be updated too, and the loop in the doctests needs to test the m = 1 case, and somewhere you need to add a doctest showing the correct behaviour for the example you gave in the ticket description!

07/11/2008 03:21:38 AM changed by wuthrich

  • attachment trac.3632.extra1.patch added.

07/11/2008 03:23:22 AM changed by wuthrich

  • summary changed from [with patch, needs work] small bug in p-adic heights to [with patch, needs more review] small bug in p-adic heights.

Ok. I added an additional patch (applied after the first) with some docstring. I hope that it is enough.

07/11/2008 06:10:02 AM changed by dmharvey

  • attachment trac.3632.extra2.patch added.

07/11/2008 06:10:45 AM changed by dmharvey

hi, I've added yet a third patch, with minor changes to your patch, plus cleaning up some existing nearby doctests which didn't make sense to me. Assuming you are happy with these changes, I think this patch is good to go.

(all three patches should be applied)

07/14/2008 02:39:02 AM changed by wuthrich

  • summary changed from [with patch, needs more review] small bug in p-adic heights to [with patch,with positive review] small bug in p-adic heights.

Yop, that is fine with me. I guess I am allowed to change the 'summary'.

07/15/2008 06:33:52 PM changed by mabshoff

  • status changed from new to closed.
  • resolution set to fixed.

Merged in Sage 3.0.6.alpha0