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.

Sign up for GitHub

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

Jump to bottom

groups: improved elimination techniques for reidemeister_ presentation #12681

Merged
merged 16 commits into from Jun 20, 2017
Merged

groups: improved elimination techniques for reidemeister_ presentation #12681

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3ad2d8b
elimination technique change
valglad May 24, 2017
decbdc6
elimination technique change
valglad May 24, 2017
730101a
improved eliminations + new FreeGroupElement methods
valglad May 27, 2017
6b3a53a
corrected doctests
valglad May 27, 2017
675720d
corrected doctests
valglad May 27, 2017
8570cd9
python3 corrections
valglad May 28, 2017
141463a
minor changes
valglad May 28, 2017
42c3496
minor change 2
valglad May 28, 2017
50f20b3
reverted subword indexing change
valglad May 30, 2017
187f2df
typo correctin
valglad May 30, 2017
df87479
further improvements
valglad Jun 2, 2017
856f590
more minor corrections
valglad Jun 7, 2017
51724b4
resolving conflicts
valglad Jun 14, 2017
6855c40
resolve conflicts
valglad Jun 19, 2017
3292969
resolve conflicts
valglad Jun 19, 2017
0a51c25
once more
valglad Jun 19, 2017
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
163 changes: 79 additions & 84 deletions sympy/combinatorics/fp_groups.py
View file
Open in desktop
Expand Up @@ -1499,45 +1499,40 @@ def elimination_technique_1(C):
rels = C._reidemeister_relators
# the shorter relators are examined first so that generators selected for
# elimination will have shorter strings as equivalent
rels.sort(reverse=True)
rels.sort()
gens = C._schreier_generators
redundant_gens = {}
contained_gens = []
redundant_rels = []
used_gens = set()
# examine each relator in relator list for any generator occuring exactly
# once
next_i = False
for i in range(len(rels) -1, -1, -1):
rel = rels[i]
# don't look for a new generator occuring once in relator which
# has already found to posses a
for gen in redundant_gens:
gen_sym = gen.array_form[0][0]
if any([gen_sym == r[0] for r in rel.array_form]):
next_i = True
break
if next_i:
next_i = False
for rel in rels:
# don't look for a redundant generator in a relator which
# depends on previously found ones
if any([g in rel.contains_generators() for g in redundant_gens]):
Copy link
Member

Choose a reason for hiding this comment

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

rel.contains_generators() could be saved in a local variable to avoid repeated recomputations.

continue
for j in range(len(gens) - 1, -1, -1):
gen = gens[j]
if rel.generator_count(gen) == 1 and gen not in contained_gens:
contained_gens = list(rel.contains_generators())
contained_gens.sort(reverse = True)
for gen in contained_gens:
if rel.generator_count(gen) == 1 and gen not in used_gens:
k = rel.exponent_sum(gen)
gen_index = rel.index(gen**k)
bk = rel.subword(gen_index + 1, len(rel))
fw = rel.subword(0, gen_index)
bk = rel.subword(gen_index + 1, len(rel)-1)
fw = rel.subword(0, gen_index-1)
chi = (bk*fw).identity_cyclic_reduction()
redundant_gens[gen] = chi**(-1*k)
contained_gens.extend(chi.contains_generators())
del rels[i]; del gens[j]
used_gens.update(chi.contains_generators())
redundant_rels.append(rel)
break
# eliminate the redundant generator from remaing relators
for i, gen in product(range(len(rels)), redundant_gens):
rels[i] = (rels[i].eliminate_word(gen, redundant_gens[gen])).identity_cyclic_reduction()
rels.sort()
rels = [r for r in rels if r not in redundant_rels]
# eliminate the redundant generators from remaining relators
rels = [r.eliminate_words(redundant_gens, _all = True).identity_cyclic_reduction() for r in rels]
rels = list(set(rels))
try:
rels.remove(C._schreier_free_group.identity)
except ValueError:
pass
gens = [g for g in gens if g not in redundant_gens]
C._reidemeister_relators = rels
C._schreier_generators = gens

Expand Down Expand Up @@ -1571,8 +1566,8 @@ def elimination_technique_2(C):
if rel.generator_count(gen) == 1:
k = rel.exponent_sum(gen)
gen_index = rel.index(gen**k)
bk = rel.subword(gen_index + 1, len(rel))
fw = rel.subword(0, gen_index)
bk = rel.subword(gen_index + 1, len(rel)-1)
fw = rel.subword(0, gen_index-1)
rep_by = (bk*fw)**(-1*k)
del rels[i]; del gens[j]
for l in range(len(rels)):
Expand All @@ -1585,11 +1580,16 @@ def elimination_technique_2(C):
def simplify_presentation(C):
"""Relies upon ``_simplification_technique_1`` for its functioning. """
rels = C._reidemeister_relators
rels_arr = _simplification_technique_1(rels)
group = C._schreier_free_group

# don't add "identity" element in relator list
C._reidemeister_relators = [group.dtype(tuple(r)).identity_cyclic_reduction() for r in rels_arr if r]
rels = list(set(_simplification_technique_1(rels)))
rels.sort()
rels = [r.identity_cyclic_reduction() for r in rels]
try:
rels.remove(C._schreier_free_group.identity)
except ValueError:
pass
C._reidemeister_relators = rels

def _simplification_technique_1(rels):
"""
Expand All @@ -1605,69 +1605,58 @@ def _simplification_technique_1(rels):
>>> F, x, y = free_group("x, y")
>>> w1 = [x**2*y**4, x**3]
>>> _simplification_technique_1(w1)
[[(x, 3)], [(x, -1), (y, 4)]]
[x**-1*y**4, x**3]

>>> w2 = [x**2*y**-4*x**5, x**3, x**2*y**8, y**5]
>>> _simplification_technique_1(w2)
[[(x, 3)], [(y, 5)], [(x, -1), (y, -2)], [(x, -1), (y, 1), (x, -1)]]
[x**-1*y*x**-1, x**3, x**-1*y**-2, y**5]

>>> w3 = [x**6*y**4, x**4]
>>> _simplification_technique_1(w3)
[[(x, 4)], [(x, 2), (y, 4)]]
[x**2*y**4, x**4]

"""
rels = list(set(rels))
rels.sort()
l_rels = len(rels)

# all syllables with single syllable
one_syllable_rels = set()
# since "nw" has a max size = l_rels, only identity element
# removal can possibly happen
nw = [None]*l_rels
for i in range(l_rels):
w = rels[i].identity_cyclic_reduction()
if w.number_syllables() == 1:

# replace one syllable relator with the corresponding inverse
# element, for ex. x**-4 -> x**4 in relator list
if w.array_form[0][1] < 0:
rels[i] = w**-1
one_syllable_rels.add(rels[i])

# since modifies the array rep., so should be
# added a list
nw[i] = list(rels[i].array_form)

# bound the exponent of relators, making use of the single
# dictionary with "gen: n" where gen^n is one of the relators
exps = {}
for i in range(len(rels)):
rel = rels[i]
if rel.number_syllables() == 1:
g = rel[0]
exp = abs(rel.array_form[0][1])
if rel.array_form[0][1] < 0:
rels[i] = rels[i]**-1
g = g**-1
sign = -1
else:
sign = 1
if not g in exps or exps[g].array_form[0][1] > exp:
exps[g] = rel**sign
Copy link
Member

Choose a reason for hiding this comment

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

Should we take the gcd of the exponents in case there is one in exps already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, yes, that would be much better.


one_syllables_words = exps.values()
# decrease some of the exponents in relators, making use of the single
# syllable relators
for i in range(l_rels):
k = nw[i]
rels_i = rels[i]
for gen in one_syllable_rels:
n = gen.array_form[0][1]
gen_arr0 = gen.array_form[0][0]
j = len(k) - 1
while j >= 0:
if gen_arr0 == k[j][0] and gen is not rels_i:
t = Mod(k[j][1], n)

# multiple of one syllable relator
if t == 0:
del k[j]
zero_mul_simp(k, j - 1)
j = len(k)

# power should be bounded by (-n/2, n/2]
elif t <= n/2:
k[j] = k[j][0], Mod(k[j][1], n)
elif t > n/2:
k[j] = k[j][0], Mod(k[j][1], n) - n
j -= 1
for i in range(len(rels)):
rel = rels[i]
if rel in one_syllables_words:
continue
rel = rel.eliminate_words(one_syllables_words, _all = True)
# if rels[i] contains g**n where abs(n) is greater than half of the power p
# of g in exps, g**n can be replaced by g**(n-p) (or g**(p-n) if n<0)
for g in rel.contains_generators():
if g in exps:
exp = exps[g].array_form[0][1]
if exp % 2 == 0:
max_exp = exp//2
else:
max_exp = exp//2 + 1
Copy link
Member

Choose a reason for hiding this comment

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

This could probably be max_exp = (exp + 1) // 2.

rel = rel.eliminate_word(g**(max_exp), g**(max_exp-exp), _all = True)
Copy link
Member

@jksuom jksuom May 28, 2017 •
edited

Choose a reason for hiding this comment

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

Does this aim to replace g**max_exp with g**(max_exp - exp)? If exp == 5 then max_exp == 2 in Python 3
and max_exp - exp == -3, which seems strange.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was working with python 2, and there if exp == 5, max_exp == 3. I've corrected the calculation of max_exp so that it works properly for python 3 as well.

rel = rel.eliminate_word(g**(-max_exp), g**(-(max_exp-exp)), _all = True)
rels[i] = rel
Copy link
Member

Choose a reason for hiding this comment

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

This seems to do in-place replacements in the argument rels. It might be safer to work with a copy to avoid surprises at the calling end. (For example, starting with rels = rels[:] should suffice.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose. It should be fine in this case because the for loop doesn't involve previously considered relators in any way and no deletions or insertions are made, but I will add it just in case in the next commit.

Copy link
Member

@jksuom jksuom May 28, 2017 •
edited

Choose a reason for hiding this comment

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

There is no problem with the for loop. I was thinking of the possibility that the caller would want to do something else with the original rels list believing that it was intact. It may not matter currently, but the situation might change in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see. Fair enough. Also, regarding the recent Travis failure: one of the tests timed out but it has nothing to do with the combinatorics module so it was probably an accident. Could you restart it?

Copy link
Member

Choose a reason for hiding this comment

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

That seems to happen in split 1/4 with Python 3.4 more often than with other versions.

rels = [r.identity_cyclic_reduction() for r in rels]
return rels

return nw

def reidemeister_presentation(fp_grp, H, elm_rounds=2, simp_rounds=2):
def reidemeister_presentation(fp_grp, H):
"""
fp_group: A finitely presented group, an instance of FpGroup
H: A subgroup whose presentation is to be found, given as a list
Expand Down Expand Up @@ -1709,9 +1698,15 @@ def reidemeister_presentation(fp_grp, H, elm_rounds=2, simp_rounds=2):
C.compress(); C.standardize()
define_schreier_generators(C)
reidemeister_relators(C)
for i in range(20):
elimination_technique_1(C)
prev_gens = []
prev_rels = []
while not set(prev_rels) == set(C._reidemeister_relators):
prev_rels = C._reidemeister_relators
while not set(prev_gens) == set(C._schreier_generators):
prev_gens = C._schreier_generators
elimination_technique_1(C)
simplify_presentation(C)
C._reidemeister_relators.sort()
C.schreier_generators = tuple(C._schreier_generators)
C.reidemeister_relators = tuple(C._reidemeister_relators)
return C.schreier_generators, C.reidemeister_relators
Expand Down