#5627: [with new patch, needs review] Trivial typo in quadratic_nonresidue
-----------------------------+----------------------------------------------
 Reporter:  kcrisman         |       Owner:  justin    
     Type:  defect           |      Status:  new       
 Priority:  trivial          |   Milestone:  sage-3.4.2
Component:  quadratic forms  |    Keywords:            
-----------------------------+----------------------------------------------

Comment(by cremona):

 I have added another patch after looking carefully at this, which does the
 following:

     1. I removed hilbert_symbol_rational(), making a trivial change to
 hilbert_symbol() so that it already works on rationals.  I think this will
 useful outside the quadratic forms module.
     2. I moved IsPadicSquare() to a member function for rationals, so you
 now say r.is_padic_square(p) instead of IsPadicSquare(r,p), while at the
 same time making the function simpler and cleaner.  I think this will also
 be useful outside the quadratic forms module.
     3. I removed random_int_upto(n) since it does the same as
 ZZ.random_element(n).
     4. I simplified quadratic_nonresidue() (and changed its name to
 least_quadratic_nonresidue()) -- by putting in three simple tests for when
 the answer is 2, 3 or 5 the loop is avoided in 7/8 of the cases.   I also
 changed the loop to "for r in xsrange(7,p)", in response to the discussion
 earlier on this ticket:  adding the x gives an iterator instead of making
 the whole list and iterating through it (bad for large p!), and adding the
 s makes the iterator yield Sage integers (so it works for p too large to
 fit into a python int).  I also added an is_prime() test on p, since
 otherwise if you give it a huge composite number there seemed to be a
 danger that it would run through a loop of length p before realising that
 the input was invalid.

 All tests in sage/quadratic_forms pass, as do those in arith.py and
 rational.py which were also touched.

 If one of the origianl posters can ok my patch then the whole lot can be
 merged, I hope.

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/5627#comment:5>
Sage <http://sagemath.org/>
Sage - Open Source Mathematical Software: Building the Car Instead of 
Reinventing the Wheel

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"sage-trac" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/sage-trac?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to