miscellaneous cleanup in perm_gps preparing for domains

[mhansen]

Apply:

1. trac_10334-permgroup_cleanup-rebase-mh.patch​

[rlm]

Looks good! Passes all long tests.

[fbissey]

I got a test failure that seems related to this patch

sage -t  "devel/sage-main/sage/groups/perm_gps/permgroup.py"
**********************************************************************
File "/usr/share/sage/devel/sage-main/sage/groups/perm_gps/permgroup.py", line 1876:
    sage: G.isomorphism_to(H)
Expected:
    Permutation group morphism:
      From: Permutation Group with generators [(2,3), (1,2,3)]
      To:   Permutation Group with generators [(1,2,4), (1,4)]
      Defn: [(2,3), (1,2,3)] -> [(2,4), (1,2,4)]
Got:
    Permutation group morphism:
      From: Permutation Group with generators [(2,3), (1,2,3)]
      To:   Permutation Group with generators [(1,2,4), (1,4)]
      Defn: [(2,3), (1,2,3)] -> [(2,4), (1,4,2)]

[vbraun]

I tripped over this patch as well (on sage-4.6.2.alpha3)

sage: S4 = SymmetricGroup(4)
sage: [ g.orbits() for g in S4.conjugacy_classes_subgroups() ]
ERROR: An unexpected error occurred while tokenizing input
The following traceback may be corrupted or invalid
The error message is: ('EOF in multi-line statement', (396, 0))

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/home/vbraun/Sage/24cell/<ipython console> in <module>()

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/misc/cachefunc.pyc in __call__(self, *args, **kwds)
    553             return self.cache[k]
    554         except KeyError:
--> 555             w = self._cachedmethod._instance_call(self._instance, *args, **kwds)
    556             self.cache[k] = w
    557             return w

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/misc/cachefunc.pyc in _instance_call(self, inst, *args, **kwds)
    776 
    777         """
--> 778         return self._cachedfunc.f(inst, *args, **kwds)
    779 
    780     def _get_instance_cache(self, inst):

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/groups/perm_gps/permgroup.pyc in orbits(self)
    952              sage: G.stabilizer(1) 
    953              Permutation Group with generators [(2,3)(4,10), (2,10,4)]
--> 954              sage: G = PermutationGroup([[(2,3,4)],[(6,7)]])
    955              sage: G.stabilizer(1)
    956              Permutation Group with generators [(6,7), (2,3,4)]

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/groups/perm_gps/permgroup.pyc in _set_gap(self)
    911          """
    912          return self._gap_().Orbit(integer).sage()
--> 913     
    914     def transversals(self, integer):
    915          """

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/groups/perm_gps/permgroup.pyc in set(self)
    896         return self._gap_().Orbits("[1..%d]" % self.largest_moved_point()).sage()
    897 
--> 898     @cached_method
    899     def orbit(self, integer):
    900          """

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__getattr__ (sage/structure/parent.c:5600)()

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.getattr_from_other_class (sage/structure/parent.c:2777)()

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.raise_attribute_error (sage/structure/parent.c:2663)()

AttributeError: 'PermutationGroup_generic_with_category' object has no attribute '_set'

For reference, I'll attach a patch that reverses the changes. With the patch reversed, I get the expected output.

[mhansen]

I've added a patch "trac_10334-permgroup_cleanup-fix-mh.patch" which fixes the issue that Volker found. The init method is cleaned up in #10335.

Let me know what the status of this ticket is.

[vbraun]

Works fine now! I can't reproduce François' doctest failure. He doesn't indicate if it is on a supported platform. We should go ahead and merge this one as it apparently works on the buildbot machines.

[fbissey]

Indeed I didn't. Turns out it is some crap I have in gap. I'll open another ticket if I find anything that should be fixed there. Nothing relevant to this ticket, it just pointed to the failure of another component.

[jdemeyer]

What's up with this ticket? It was merged in sage-4.6.2.alpha2 and worked fine on the buildbot.

I'm going to unmerge this in sage-4.6.2.alpha4 until this is sorted out.

Volker: it is a very bad idea to reopen tickets which are already merged, this will cause major confusion. At the very least, it should be discussed with the release manager (me in this case).

[mhansen]

It just needs trac_10334-permgroup_cleanup-mh.patch and trac_10334-permgroup_cleanup-fix-mh.patch to be applied. The first was already merged in alpha2.

[jdemeyer]

Please change the commit messages of the patches to something more descriptive (and keep in mind that the ticket number should be on the first line of the commit message).

[mhansen]

I've fixed the commit message and combined the two patches into one.

[jdemeyer]

Doctest error on 64-bit OS X 10.6:

sage -t -long  -force_lib devel/sage/sage/groups/perm_gps/permgroup.py
**********************************************************************
File "/Users/buildbot/build/sage/bsd-1/bsd_64_full/build/sage-4.6.2.rc0/devel/sage-main/sage/groups/perm_gps/permgroup.py", line 1174:
    sage: G.random_element()
Expected:
    (1,2)(4,5)
Got:
    (2,3)(4,5)
**********************************************************************

[rbeezer]

FWIW, I applied this to 4.7.alpha1 with a single (easy-to-fix) hunk failure. I could post a rebase if that is desirable.

Passed all tests in sage/groups, and in particular I did not see the "random element" failure noted above (64-bit Ubuntu).

[rbeezer]

On 4.7.rc0.

1) 3 hunk failures when applying trac_10334-permgroup_cleanup-rebase-mh.patch, a rebase is attached.

2) With rebase a quotient group fails in a doctest. Here's the evidence:

Clean 4.7.rc0:

G = DihedralGroup(20)
C = G.commutator()
Q = G.quotient(C)
Q.is_abelian()

4.7.rc0 with rebased patch:

sage: G = DihedralGroup(20)
sage: C = G.commutator()
sage: Q = G.quotient(C)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/sage/sage-4.7.rc0/devel/sage-main/<ipython console> in <module>()

/sage/sage-4.7.rc0/local/lib/python2.6/site-packages/sage/groups/perm_gps/permgroup.pyc in quotient(self, N)
   1483         phi = Q.RegularActionHomomorphism()
   1484         gens = phi.Image().GeneratorsOfGroup()
-> 1485         return PermutationGroup([self(gen) for gen in gens])
   1486
   1487     def quotient_group(self, N):

/sage/sage-4.7.rc0/local/lib/python2.6/site-packages/sage/groups/perm_gps/permgroup.pyc in __call__(self, x, check)
    593             return self.identity()
    594
--> 595         return self._element_class()(x, self, check=check)
    596
    597     def _coerce_impl(self, x):

/sage/sage-4.7.rc0/local/lib/python2.6/site-packages/sage/groups/perm_gps/permgroup_element.so in sage.groups.perm_gps.permgroup_element.PermutationGroupElement.__init__ (sage/groups/perm_gps/permgroup_element.c:3008)()

TypeError: permutation (1, 2)(3, 4) not in Dihedral group of order 40 as a permutation group

I've tried to locate the error, with no luck. My rebase could be at fault, but I've double-checked that as well.

[rbeezer]

Forgot some details above.

On clean 4.7.rc0 the "is_abelian" returns {{{True}}. This is a doctest that has been passing in alpha testing.

On patched version, the commutator is a subgroup, and it is normal:

sage: G = DihedralGroup(20)
sage: C = G.commutator()
sage: C.is_subgroup(G)
True
sage: C.is_normal(G)
True

[nthiery]

Fixed a remaining doctest with Rob.

[rbeezer]

For the record, it looks like quotients were failing because elements of the permutation group representation of the quotient were being coerced into the larger of the two groups used in the quotient construction. Since the representation of the quotient has little connection with this larger group the changes to the __init__ method would now notice and fail.

[rbeezer]

Passes all tests on 4.7.rc0 and looks good.

Lots of good improvements and fixes in here. Thanks, Mike.

[mhansen]

Test comment.

[mhansen]

And another test comment.