Hi Craig!
Thanks for investigating this in detail!
On Fri, Feb 05, 2010 at 02:44:59AM -0800, Craig Citro wrote:
> Yep, using ZZ as a parent for something which isn't of class
> sage.rings.integer.Integer is what was causing the segfault here.
> There's actually a bit more to the story -- in particular, one should
> wonder why it only segfaulted on OSX. I'll make some comments about
> what was going on below; more importantly, though, is what we should
> do about fixing it. In general, there are a lot of parents that assume
> that elements with that parent are of some fixed type; breaking this
> rule would be bad on two counts: (1) we'd pay the price by having to
> do a bunch of typechecks everywhere, which is no good, and (2) the
> logic that depends on this isn't clearly defined -- it's just an
> *implicit* assumption in lots of code all over the place.
That's indeed a reasonable assumption, and I would not mind if
breaking this assumption would result in an exception, or some mild
unpredictable result. On the other hand, I am uncomfortable with
getting a segfault by writing rather innocent looking pure Python
code, especially if the segfault is non systematic or platform
dependent.
Just a quick rant. Here the thing is that we have an invariant for:
self.parent() = ZZ <=> self.__class__ = Integer
When some low-level method of IntegerRing / IntegerRing needs to check
than an object is an integer, and don't want to test both, wouldn't it
be safer (and as fast?) to test that the class of the object is
Integer, and then assume that its parent is ZZ, rather than the
converse? That sounds safer to me, because breaking the assumption
requires doing something wrong with the class Integer, which is well
localized. Whereas any element far far away in user's code can wrongly
set its parent to IntegerRing.
That being said, all of this is far from my area of expertise, and I
let you judge for the best.
> So how should we fix it? Robert Bradshaw pointed out that there should
> probably be a corresponding ParentWrapper, that one could use to
> create wrapped parents for the wrapped elements. In fact, I think we
> should go one step further -- I don't see why you should be able to
> end up with an ElementWrapper without the corresponding ParentWrapper.
> So passing parent=P should probably just create a wrapper out of P, if
> it isn't one already. In general, being able to just choose your
> parent out of the blue is a dangerous thing ... this might be a
> reasonably controlled way to do so.
For all practical use cases, the purpose of ElementWrapper is to
construct an element class for a specific parent that one is
implementing (see the category examples for extensive use
cases). Those parents are not wrapping anything in particular. The
fact that the data structure of an element consists of an integer does
not mean that the parent has anything to do with IntegerRing.
To make it short: the use of ZZ in the doctest is purely for testing
purposes, and does not reflect any practical use cases. Florent's fix
is good (thanks Florent by the way).
Cheers,
Nicolas
--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/
--
To post to this group, send an email to sage-devel@googlegroups.com
To unsubscribe from this group, send an email to
sage-devel+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org
