Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert between free group elements considering generator names. #14684

Closed
miguelmarco opened this issue Jun 4, 2013 · 18 comments
Closed

Convert between free group elements considering generator names. #14684

miguelmarco opened this issue Jun 4, 2013 · 18 comments

Comments

@miguelmarco
Copy link
Contributor

Right now, when we try to convert elements between free groups, they just get converted to the Tietze list, forgeting about the names of the generators. This can cause strange behaviour like:

sage: F.<a,b>=FreeGroup()
sage: G.<b,a>=FreeGroup()
sage: F(a) 
b

This patch solves this, looking for generators with matching names.

CC: @vbraun @sagetrac-sydahmad @videlec @jhpalmieri @sagetrac-tjolivet @rbeezer @dimpase @sagetrac-dshurbert

Component: group theory

Keywords: free groups

Author: Miguel Marco

Branch/Commit: 4a479ed

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/14684

@miguelmarco
Copy link
Contributor Author

comment:1

Attachment: 14684_free_group_conversion.patch.gz

@miguelmarco miguelmarco assigned miguelmarco and unassigned wdjoyner Jun 5, 2013
@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

Branch: public/14684

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

comment:9

Hello,

I created a branch from the ticket with sage -dev. The review can starts from there.

What's the point of the following code in _element_constructor_?

if hasattr(P, '_freegroup_'):
    if P.FreeGroup() is self:
        return self.element_class(self, x.Tietze(), **kwds)

As far as I looked, there is no object in Sage with a _freegroup_ attribute.

Vincent


New commits:

b2d253dTrac #14684: make conversion between free groups aware of generator names

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

Commit: b2d253d

@miguelmarco
Copy link
Contributor Author

comment:10

It is an error. It should be _free_group (finitely presented groups have that attribute, that contains the free group after which they are defined).

Apparently it works even without that because the element_class init first tries to get the Tietze list anyways.

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

comment:11

So please, fix that!

@miguelmarco
Copy link
Contributor Author

Changed branch from public/14684 to u/mmarco/ticket/14684

@miguelmarco
Copy link
Contributor Author

Changed branch from u/mmarco/ticket/14684 to public/14684

@miguelmarco
Copy link
Contributor Author

comment:14

Fixed.

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

Changed branch from public/14684 to u/mmarco/ticket/14684

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

Changed commit from b2d253d to 4a479ed

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

comment:15

Thanks. If it is, your commit should appear on this page... and it does not. I only see your commit at u/mmarco/ticket/14684 (public/14684 is one commit back). I switch back to the name of the branch that you used. If you want to commit to a specific branch use git push trac HEAD:public/14684.

Vincent


New commits:

4a479edRemved unnecessary check for _freegroup_

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

Reviewer: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Aug 25, 2014

comment:16

Perfect!

Vincent

@vbraun
Copy link
Member

vbraun commented Aug 25, 2014

comment:17

Author name should be real name, not trac username

@miguelmarco
Copy link
Contributor Author

Changed author from mmarco to Miguel Marco

@miguelmarco
Copy link
Contributor Author

comment:18

Sorry, i always forget those details.

@vbraun
Copy link
Member

vbraun commented Aug 26, 2014

Changed branch from u/mmarco/ticket/14684 to 4a479ed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants