Ticket #13101 (needs_review defect)

Opened 11 months ago

Last modified 6 weeks ago

enumerate_totallyreal_fields bug fix

Reported by: jvoight Owned by: davidloeffler
Priority: major Milestone: sage-5.10
Component: number fields Keywords:
Cc: Work issues:
Report Upstream: N/A Reviewers:
Authors: Robert Harron, John Voight Merged in:
Dependencies: Stopgaps:

Description

sage: enumerate_totallyreal_fields_all(8, 108)


IndexError? Traceback (most recent call last)

/home/jvoight/sage-4.2/devel/sage-main/sage/rings/number_field/<ipython console> in <module>()

/usr/local/share/sage-5.0/local/lib/python2.7/site-packages/sage/rings/number_field/totallyreal_rel.pyc in enumerate_totallyreal_fields_all(n, B, verbose, return_seqs)

887 print "Taking F =", Sds[i][1] 888 F = NumberField?(ZZx(Sds[i][1]), 't')

--> 889 T = enumerate_totallyreal_fields_rel(F, n/d, B, verbose=verbose, return_seqs=return_seqs)

890 if return_seqs: 891 for i in range(3):

/usr/local/share/sage-5.0/local/lib/python2.7/site-packages/sage/rings/number_field/totallyreal_rel.pyc in enumerate_totallyreal_fields_rel(F, m, B, a, verbose, return_seqs)

726 T.incr(f_out,verbose) 727 else:

--> 728 T.incr(f_out)

729 730 Fx = PolynomialRing?(F, 'xF')

/usr/local/share/sage-5.0/local/lib/python2.7/site-packages/sage/rings/number_field/totallyreal_rel.pyc in incr(self, f_out, verbose, haltk)

542 # New bounds from Lagrange multiplier in degree 3.

543 bminmax = [lagrange_degree_3(m,v(self.a[m-1]),v(self.a[m-2]),v(self.a[m-3])) for v in self.Foo]

--> 544 self.b_lower = [bminmax[i][0] for i in range(len(bminmax))]

545 self.b_upper = [bminmax[i][1] for i in range(len(bminmax))] 546

IndexError?: list index out of range

The fix is easy: we just need to add the lines

if len(z4minmax) < 1:

z4minmax = [0.0, -1.0]

[ return z4minmax ]

before line 295 of sage/rings/number_field/totallyreal_data.pyx. (Yes, I'm still intimidated by creating a patch.)

The issue was that when the lagrange multipliers gives contradictory bounds (a good thing, since it says the enumeration can stop in that branch), it was not sending a pair of bounds, just a single element, causing a type error.

JV

Attachments

trac_13101_bug_in_enumerate_totallyreal_fields_all.patch Download (1.5 KB) - added by robharron 6 weeks ago.

Change History

Changed 6 weeks ago by robharron

comment:1 Changed 6 weeks ago by robharron

  • Status changed from new to needs_review
  • Reviewers citro deleted

Okay, I made a patch with the change you mentioned (I think I figured out what indentation you meant, but double-check to make sure). The output of this command should be the empty array, right? That's what I gather from the data on your website/your paper.

(Also, I fixed a typo).

comment:2 Changed 6 weeks ago by robharron

  • Authors set to Robert Harron, John Voight

comment:3 Changed 6 weeks ago by jvoight

Thanks for doing this and for fixing the typo! Yes, the tabbing/formatting is correct. Am I allowed to review this? JV

comment:4 Changed 6 weeks ago by robharron

Hmm, I don't really know how that works. I mean, if you did, you'd probably want to remove your name from the authors... You could try, what's the worse that could happen?!

Note: See TracTickets for help on using tickets.