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 -~----------~----~----~----~------~----~------~--~---