Very good.  Thank you for taking a look at the code -- and no offense 
taken: I just didn't know what to do next!

I will leave the is_irreducible() bug/definition for #5140 and create a 
new ticket for is_primitive including the comments from John and 
C.Witty.  I'll leave it unassigned for now.

Thanks!

- Ryan

John Cremona wrote:
> PS I found that #5140 already addresses related issues, and contains a
> patch by me for the is_irreducible() bug.  the patch needs review, and
> that explains why (1) I remembered already fixing it and (2) why it is
> not fixed.
> 
> I will rebase that patch to 3.4 and then invite reviewers.
> 
> John
> 
> 2009/3/18 John Cremona <john.crem...@gmail.com>:
>> I hope I did not offend anyone, least of all someone who had provided
>> a patch which makes a useful efficiency improvement!  But as Martin
>> invited all sage-devel to look at that code, I did!
>>
>> I'll be happy to provide a patch for the bug in is_irreducible for
>> rational polys.  (I call it a bug, though it's actually a correct
>> implementation of a wrong definition).
>>
>> What Ryan suggest for is_primitive might be a good way to go;  as far
>> as I know the meaning which is relevant here, namely "irreducible and
>> the root generates the multiplicative group of the extension" is only
>> relevant over finite fields (and no other fields).  The other meaning
>> (coprime coefficients) is certainly not very useful over fields as it
>> is the same as "non-zero", so we are left with the question "what, if
>> anything, should we take is_primitive() to mean for polynomials in
>> F[x] where F is an infinite field?"
>>
>> John
>>
>> 2009/3/18 Ryan Hinton <iob...@email.com>:
>>> Unfortunately, I don't know what on earth is_primitive() is doing there.
>>>   I didn't put it there.  I wrote the patch to as a performance
>>> enhancement to the _existing_ is_primitive implementation.  is_primitive
>>> was there already, so the current ticket is probably not the best place
>>> to discuss its existence or placement.  (And I didn't even touch
>>> is_irreducible.)
>>>
>>> C.Witty suggested on IRC that #5535 be accepted as is, and to open
>>> additional tickets to address John's concerns (is_primitive placement
>>> and is_irreducible bug).  He also suggested that, given the differing
>>> meanings of "primitive polynomial" in ring theory vs. field theory, one
>>> way to resolve the issue is to split up the polynomial classes into
>>> "polynomials over fields" and "polynomials over rings."  Then these new
>>> base classes (and/or their derived classes) can implement is_primitive
>>> as appropriate.
>>>
>>> Unfortunately, I'm not enough of a mathematician to address the problems
>>> (I've learned most of what I know about these issues from Wikipedia over
>>> the last few days), so I'll assign the new trac tickets to John for now.
>>>
>>> In the mean time, does anyone object to the patch for trac #5535 that
>>> gives a huge speed improvement but allows a user to get garbage out if
>>> they put garbage in?  :-)
>>>
>>> Thanks!
>>>
>>> - Ryan
>>>
>>> John Cremona wrote:
>>>> What on earth is that function is_primitive() doing there?  If you
>>>> asked me to define what it means for a univariate polynomial over a
>>>> ring to be primitive then I would say that it means that the
>>>> coefficients generate the unit ideal.
>>>>
>>>> The function there seems to be a different concept only relevant for
>>>> polynomials over finite fields.   So why is it in class Polynomial?
>>>>
>>>> It gets worse:
>>>>
>>>> sage: R.<x> = QQ[]
>>>> sage: f=3*x^2-6
>>>> sage: f.is_irreducible()
>>>> False
>>>>
>>>> This is WRONG.  I thought I fixed that months ago, but here it is
>>>> again.  The implementation
>>>>
>>>>     def is_irreducible(self):
>>>>         S = PolynomialRing(ZZ, self.variable_name())
>>>>         return S(self.denominator()*self).is_irreducible()
>>>>
>>>> would fail any first year undergraduate exam I was responsible for.
>>>>
>>>> John
>>>>
>>>> 2009/3/16 Martin Albrecht <m...@informatik.uni-bremen.de>:
>>>>> Hi there,
>>>>>
>>>>>  http://trac.sagemath.org/sage_trac/ticket/5535
>>>>>
>>>>> adds a neat way of shooting yourself in the foot in the name of 
>>>>> performance,
>>>>> so I wonder if anyone has any hard feelings about that? I suggested to
>>>>> include this in Sage (Ryan had a local version for his application), so I
>>>>> think it is worth it.
>>>>>
>>>>> Thoughts?
>>>>> Martin
>>>>> --
>>>>> name: Martin Albrecht
>>>>> _pgp: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x8EF0DC99
>>>>> _otr: 47F43D1A 5D68C36F 468BAEBA 640E8856 D7951CCF
>>>>> _www: http://www.informatik.uni-bremen.de/~malb
>>>>> _jab: martinralbre...@jabber.ccc.de
>>>>>
>>>>>
> 
> > 

--~--~---------~--~----~------------~-------~--~----~
To post to this group, send email to sage-devel@googlegroups.com
To unsubscribe from this group, send email to 
sage-devel-unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URLs: http://www.sagemath.org
-~----------~----~----~----~------~----~------~--~---

Reply via email to