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

Added method to calculate Abelian Invariants #16670

Merged
merged 11 commits into from May 13, 2019
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions sympy/combinatorics/fp_groups.py
Expand Up @@ -527,6 +527,17 @@ def is_cyclic(self):
"is not implemented")
return P.is_cyclic

def abelian_invariants(self):
"""
Returns Abelian Invariants of a group.
"""
try:
P, T = self._to_perm_group()
except NotImplementedError:
raise NotImplementedError("abelian invariants is not implemented"
"for infinite group")
return P.abelian_invariants()


class FpSubgroup(DefaultPrinting):
'''
Expand Down
76 changes: 76 additions & 0 deletions sympy/combinatorics/perm_groups.py
Expand Up @@ -3,6 +3,7 @@
from random import randrange, choice
from math import log
from sympy.ntheory import primefactors
from sympy import multiplicity

from sympy.combinatorics import Permutation
from sympy.combinatorics.permutations import (_af_commutes_with, _af_invert,
Expand Down Expand Up @@ -116,6 +117,8 @@ class PermutationGroup(Basic):

.. [13] http://www.math.colostate.edu/~hulpke/CGT/cgtnotes.pdf

.. [14] https://www.gap-system.org/Manuals/doc/ref/manual.pdf

"""
is_group = True

Expand Down Expand Up @@ -1702,6 +1705,79 @@ def is_abelian(self):
return False
return True

def abelian_invariants(self):
"""
Returns the abelian invariants for the given group.
Let ``G`` be a nontrivial finite abelian group. Then G is isomorphic to
the direct product of finitely many nontrivial cyclic groups of
prime-power order.

The prime-powers that occur as the orders of the factors are uniquely
determined by G. More precisely, the primes that occur in the orders of the
factors in any such decomposition of ``G`` are exactly the primes that divide
``|G|`` and for any such prime ``p``, if the orders of the factors that are
p-groups in one such decomposition of ``G`` are ``p^{t_1} >= p^{t_2} >= ... p^{t_r}``,
then the orders of the factors that are p-groups in any such decomposition of ``G``
are ``p^{t_1} >= p^{t_2} >= ... p^{t_r}``.

The uniquely determined integers ``p^{t_1} >= p^{t_2} >= ... p^{t_r}``, taken
for all primes that divide ``|G|`` are called the invariants of the nontrivial
group ``G`` as suggested in ([14], p. 542).

Notes
=====

We adopt the convention that the invariants of a trivial group are [].

Examples
========

>>> from sympy.combinatorics import Permutation
>>> from sympy.combinatorics.perm_groups import PermutationGroup
>>> a = Permutation([0, 2, 1])
>>> b = Permutation([1, 0, 2])
>>> G = PermutationGroup([a, b])
>>> G.abelian_invariants()
[2]
>>> from sympy.combinatorics.named_groups import CyclicGroup
>>> G = CyclicGroup(7)
>>> G.abelian_invariants()
[7]

"""
if self.is_trivial:
return []
gns = self.generators
inv = []
der = self.derived_subgroup()
G = self

for p in primefactors(G.order()):
ranks = []
r = 2
while r > 1:
H = der
l = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using something more informative like pows instead of l (that may sometimes look like 1)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right I myself got confused between 1 and l several times.

for g in gns:
elm = g**p
if not H.contains(elm):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contains is a fairly expensive method since a coset factorization has to be computed. I think that the test should be taken with the same (fixed) H for all powers g**p. That filtered list of powers could probably also used as gens for the next round of the r loop instead of G.generators that is actually H.generators and thus contains also generators of der.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By fixed H you mean we should avoid H = der? and use the same H as computed in the previous iteration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that H should not be redefined for each g in gns (whose power is not contained in it). Instead, we should collect those powers g**p that are not in H in a list and then update H by adding all those new generators at once instead of one at a time. (So H is fixed during one for loop.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I got it!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!, please have a look.

l.append(elm)
H = PermutationGroup(H.generators + l)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor optimization:

 H = PermutationGroup(H.generators + l) if l else H

r = G.order()//H.order()
G = H
gns = G.generators
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gns now contains also the generators of der. Would it suffice to take only those that are in the list l of powers?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked and yes it's consistent with the previous result.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, I would suggest that the creation of l above be changed. Maybe something like this could work:

pows = [g**p for g in gns]
gns = [pow in pows if not H.contains(pow)]

It should be possible to use the name gns (or maybe gens) at this point already.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the above gns should be
gns = [pow for pow in pows if not H.contains(pow)]
and in that case does it really make a difference, for me both(current and the above mentioned changes) looks same.

Copy link
Member

@jksuom jksuom Apr 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, of course. I did forget to add the for part.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After applying above changes one of the test-case seems to fail i.e the result is not consistent with GAP.

>>> G = AbelianGroup(2,3,4)
>>> assert G.abelian_invariants() == [2, 3, 4]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError
>>> G.abelian_invariants()
[2, 4]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this H = PermutationGroup(H.generators + l) should have dir.generators instead of the changing H.generators.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

H = PermutationGroup(H.generators + l) works fine the above failure is encountered after below changes:

pows = [g**p for g in gns]
gns = [pow for pow in pows if not H.contains(pow)]

And the reason seems to be same that you have mentioned above after calculating pows we are updating H = PermutationGroup(H.generators + pows) where H.generators is always der.generators.

So, I think the current commit is okay!

if r != 1:
ranks.append(multiplicity(p, r))

if ranks:
l = [1]*ranks[0]
for i in ranks:
for j in range(0, i):
l[j] = l[j]*p
inv.extend(l)
inv.sort()
return inv

def is_elementary(self, p):
"""Return ``True`` if the group is elementary abelian. An elementary
abelian group is a finite abelian group, where every nontrivial
Expand Down
10 changes: 10 additions & 0 deletions sympy/combinatorics/tests/test_fp_groups.py
Expand Up @@ -241,3 +241,13 @@ def test_cyclic():
assert f.is_cyclic
f = FpGroup(F, [x**4, y**2, x*y*x**-1*y])
assert not f.is_cyclic


def test_abelian_invariants():
F, x, y = free_group("x, y")
f = FpGroup(F, [x*y, x**-1*y**-1*x*y*x])
assert f.abelian_invariants() == []
f = FpGroup(F, [x*y, x*y**-1])
assert f.abelian_invariants() == [2]
f = FpGroup(F, [x**4, y**2, x*y*x**-1*y])
assert f.abelian_invariants() == [2, 4]
19 changes: 19 additions & 0 deletions sympy/combinatorics/tests/test_perm_groups.py
Expand Up @@ -957,3 +957,22 @@ def test_cyclic():
assert G.is_cyclic
G = AlternatingGroup(4)
assert not G.is_cyclic


def test_abelian_invariants():
G = AbelianGroup(2,3,4)
assert G.abelian_invariants() == [2, 3, 4]
G=PermutationGroup([Permutation(1,2,3,4), Permutation(1,2), Permutation(5,6)])
assert G.abelian_invariants() == [2, 2]
G = AlternatingGroup(7)
assert G.abelian_invariants() == []
G = AlternatingGroup(4)
assert G.abelian_invariants() == [3]
G = DihedralGroup(4)
assert G.abelian_invariants() == [2, 2]

G = PermutationGroup([Permutation(1,2,3,4,5,6,7)])
assert G.abelian_invariants() == [7]
G = DihedralGroup(12)
S = G.sylow_subgroup(3)
assert S.abelian_invariants() == [3]