Opened 8 years ago

Closed 8 years ago

#17246 closed defect (fixed)

FreeGroup(0)([]) broken

Reported by: Darij Grinberg Owned by:
Priority: minor Milestone: sage-6.4
Component: group theory Keywords: gap, group, free group, border case
Cc: Travis Scrimshaw Merged in:
Authors: Travis Scrimshaw Reviewers: Darij Grinberg
Report Upstream: N/A Work issues:
Branch: 40c165c (Commits, GitHub, GitLab) Commit: 40c165c510dcabee50b2e7e65b6a6a2ea6d16611
Dependencies: Stopgaps:

Status badges

Description

This is a border case, but I'd still prefer it to work:

sage: F = FreeGroup(0)
sage: F([])
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-47-49d58f580ab1> in <module>()
----> 1 F([])

/scratch/sage-6.4.beta2/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9603)()

/scratch/sage-6.4.beta2/local/lib/python2.7/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4256)()

/scratch/sage-6.4.beta2/local/lib/python2.7/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4163)()

/scratch/sage-6.4.beta2/local/lib/python2.7/site-packages/sage/groups/free_group.pyc in _element_constructor_(self, *args, **kwds)
    808             P = x.parent()
    809         except AttributeError:
--> 810             return self.element_class(self, x, **kwds)
    811         if isinstance(P, FreeGroup_class):
    812             names = set(P._names[abs(i)-1] for i in x.Tietze())

/scratch/sage-6.4.beta2/local/lib/python2.7/site-packages/sage/groups/free_group.pyc in __init__(self, parent, x)
    225                     i=i+1
    226             AbstractWordTietzeWord = libgap.eval('AbstractWordTietzeWord')
--> 227             x = AbstractWordTietzeWord(l, parent._gap_gens())
    228         ElementLibGAP.__init__(self, parent, x)
    229 

/scratch/sage-6.4.beta2/local/lib/python2.7/site-packages/sage/libs/gap/element.so in sage.libs.gap.element.GapElement_Function.__call__ (build/cythonized/sage/libs/gap/element.c:15598)()

ValueError: libGAP: Error, List Element: <list>[1] must have an assigned value

Change History (9)

comment:1 Changed 8 years ago by Travis Scrimshaw

Cc: Travis Scrimshaw added

I will get a fix in a day or two.

comment:2 Changed 8 years ago by Travis Scrimshaw

Authors: Travis Scrimshaw
Branch: public/groups/trivial_free_group-17246
Commit: 9121d13a280b9b027418e6f1421e049ee17bbdea
Status: newneeds_review

I made the input to element construct just return the identity anytime a bool(x) is False (which includes empty lists and tuples), so it fixes the issue (and possibly slightly faster than before).


New commits:

9121d13Fix for trivial free group.

comment:3 Changed 8 years ago by Darij Grinberg

Looks good to *me*, though a shade of doubt remains about whether not x doesn't encompass a few cases too many. Can you run this by someone who knows Sage's free groups well?

Version 0, edited 8 years ago by Darij Grinberg (next)

comment:4 Changed 8 years ago by git

Commit: 9121d13a280b9b027418e6f1421e049ee17bbdeac65cb5eb15ef5f1437debb3fbaaf55b8e1540585

Branch pushed to git repo; I updated commit sha1. New commits:

c65cb5ePrevent 0 from being used to construct the identity.

comment:5 Changed 8 years ago by git

Commit: c65cb5eb15ef5f1437debb3fbaaf55b8e154058540c165c510dcabee50b2e7e65b6a6a2ea6d16611

Branch pushed to git repo; I updated commit sha1. New commits:

40c165cChanged test to explicitly check x == [] or x == ().

comment:6 Changed 8 years ago by Travis Scrimshaw

It did overextend to include 0 going to the identity. However the input is suppose to be a list or tuple whose entries correspond to (inverse) generators, so I've changed it to explicitly check x == [] or x == ().

comment:7 Changed 8 years ago by Darij Grinberg

Status: needs_reviewpositive_review

Thanks!

comment:8 Changed 8 years ago by Travis Scrimshaw

Reviewers: Darij Grinberg

NP. Thanks for doing the review.

comment:9 Changed 8 years ago by Volker Braun

Branch: public/groups/trivial_free_group-1724640c165c510dcabee50b2e7e65b6a6a2ea6d16611
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.