-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add automaton for word reduction in rewriting systems #14735
Changes from 24 commits
db6c210
809ac89
a7b93a9
f15eecd
3ee1d07
13736ae
cde6408
c2c57b2
5898726
71bb68a
173c6c1
1aa4f01
dfe5122
2695919
4bb64d3
5716eb9
e3969c1
bfbb650
51f58ce
f20a178
57c95a2
930cfa9
4fb9964
424f89f
8935d87
d4d5741
96d425e
14e5686
e58652e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
from sympy import S | ||
from sympy.combinatorics.free_groups import FreeGroupElement | ||
from sympy.combinatorics.rewritingsystem_fsm import State, StateMachine | ||
|
||
class RewritingSystem(object): | ||
''' | ||
|
@@ -31,11 +32,19 @@ def __init__(self, group): | |
# at any point | ||
self._max_exceeded = False | ||
|
||
# Reduction automaton | ||
self.reduction_automaton = None | ||
self._new_rules = {} | ||
|
||
# dictionary of reductions | ||
self.rules = {} | ||
self.rules_cache = deque([], 50) | ||
self._init_rules() | ||
|
||
# Create a finite state machine as an instance of the StateMachine object | ||
self.reduction_automaton = StateMachine('fsm') | ||
self.construct_automaton() | ||
|
||
def set_max(self, n): | ||
''' | ||
Set the maximum number of rules that can be defined | ||
|
@@ -74,6 +83,9 @@ def _add_rule(self, r1, r2): | |
self._max_exceeded = True | ||
raise RuntimeError("Too many rules were defined.") | ||
self.rules[r1] = r2 | ||
# Add the newly added rule to the `new_rules` dictionary. | ||
if self.reduction_automaton: | ||
self._new_rules[r1] = r2 | ||
|
||
def add_rule(self, w1, w2, check=False): | ||
new_keys = set() | ||
|
@@ -285,3 +297,187 @@ def reduce(self, word, exclude=None): | |
if new != prev: | ||
again = True | ||
return new | ||
|
||
def _compute_inverse_rules(self, rules): | ||
''' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be part of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to make use of this method in the |
||
Compute the inverse rules for a given set of rules. | ||
The inverse rules are used in the automaton for word reduction. | ||
|
||
Arguments: | ||
rules (dictionary): Rules for which the inverse rules are to computed. | ||
|
||
Returns: | ||
Dictionary of inverse_rules. | ||
|
||
''' | ||
inverse_rules = {} | ||
for r in rules: | ||
rule_key_inverse = r**-1 | ||
rule_value_inverse = (rules[r])**-1 | ||
if (rule_value_inverse < rule_key_inverse): | ||
inverse_rules[rule_key_inverse] = rule_value_inverse | ||
else: | ||
inverse_rules[rule_value_inverse] = rule_key_inverse | ||
return inverse_rules | ||
|
||
def construct_automaton(self): | ||
''' | ||
Construct the automaton based on the set of reduction rules of the system. | ||
|
||
Automata Design: | ||
The accept states of the automaton are the proper prefixes of the left hand side of the rules. | ||
The complete left hand side of the rules are the dead states of the automaton. | ||
|
||
''' | ||
self._add_to_automaton(self.rules) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you do this, then you'll still have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By the time the control flow reaches here, the |
||
|
||
def _add_to_automaton(self, rules): | ||
''' | ||
Add new states and transitions to the automaton. | ||
|
||
Summary: | ||
States corresponding to the new rules added to the system are computed and added to the automaton. | ||
Transitions in the previously added states are also modified if necessary. | ||
|
||
Arguments: | ||
rules (dictionary) -- Dictionary of the newly added rules. | ||
|
||
''' | ||
# Automaton vairables | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. variables |
||
automaton_alphabet = [] | ||
proper_prefixes = {} | ||
|
||
# compute the inverses of all the new rules added | ||
all_rules = rules | ||
inverse_rules = self._compute_inverse_rules(all_rules) | ||
all_rules.update(inverse_rules) | ||
|
||
# All the transition symbols in the automaton | ||
generators = list(self.alphabet) | ||
generators += [gen**-1 for gen in generators] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going to be the same every time - I think it would make sense for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these 2 lines can be removed if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need them for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
# Keep track of the accept_states. | ||
accept_states = [] | ||
|
||
# The symbols present in the new rules are the symbols to be verified at each state. | ||
# computes the automaton_alphabet, as the transitions solely depend upon the new states. | ||
for rule in all_rules: | ||
automaton_alphabet += rule.letter_form_elm | ||
automaton_alphabet = set(automaton_alphabet) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you should make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. But in this case, the transitions of the old states solely depend on the symbols of the new rules. For example, if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see. In that case what you should do here depends on what you think about the thing I suggested for adding new transitions later. If you decide to construct a dictionary similar to what I described, then you could extract the letters you want from there. |
||
|
||
# Compute the proper prefixes for every rule. | ||
for r in all_rules: | ||
proper_prefixes[r] = [] | ||
letter_word_array = [s for s in r.letter_form_elm] | ||
for i in range (1, len(letter_word_array)): | ||
letter_word_array[i] = letter_word_array[i-1]*letter_word_array[i] | ||
proper_prefixes[r] = letter_word_array | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this and pretty much the rest of this function is almost an exact copy of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered this? I think it would work quite well and the code would be more compact. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I've updated this. |
||
|
||
# Add dead states. | ||
# This is essentially checking the left hand side of the rules. | ||
for rule in all_rules: | ||
if not rule in self.reduction_automaton.states: | ||
self.reduction_automaton.add_state(rule, state_type='d', rh_rule=all_rules[rule]) | ||
|
||
for r in all_rules: | ||
prop_prefix = proper_prefixes[r] | ||
for elem in prop_prefix: | ||
if not elem in self.reduction_automaton.states: | ||
self.reduction_automaton.add_state(elem, state_type='a') | ||
accept_states.append(elem) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Okay, I know I said that there shouldn't be any more changes to make but now that I've noticed multiple small inefficiencies in this part of the code, I can't just let them pass. I promise what I suggest here will be the absolutely last changes and this will be good to merge) The last 4 for loops all iterate over |
||
|
||
# Add new transitions for every state. | ||
for state in self.reduction_automaton.states: | ||
current_state_name = state | ||
current_state_type = self.reduction_automaton.states[state].state_type | ||
# Transitions will be modified only when current_state belongs to the proper_perfixes of the new rules. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
# The rest are ignored if they cannot lead to a dead state after a finite number of transisitons. | ||
if current_state_type == "start": | ||
for letter in automaton_alphabet: | ||
if letter in self.reduction_automaton.states: | ||
self.reduction_automaton.states[state].add_transition(letter, letter) | ||
else: | ||
self.reduction_automaton.states[state].add_transition(letter, current_state_name) | ||
else: | ||
# Store subwords of current state in an array. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
current_state_name_array = [] | ||
next_sub = current_state_name | ||
while len(next_sub): | ||
current_state_name_array.append(next_sub) | ||
next_sub = next_sub.subword(1, len(next_sub)) | ||
# Check if the transition to any new state in posible. | ||
for sub_word in current_state_name_array: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to store the suffixes in |
||
if sub_word in accept_states: | ||
# Add accept states. | ||
if current_state_type == "accept": | ||
for letter in automaton_alphabet: | ||
next = current_state_name*letter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
while True: | ||
if len(next) <= 1: | ||
if next in self.reduction_automaton.states: | ||
self.reduction_automaton.states[state].add_transition(letter, next) | ||
else: | ||
self.reduction_automaton.states[state].add_transition(letter, 'start') | ||
break | ||
else: | ||
if next in self.reduction_automaton.states: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could replace
once right after the loop. This is equivalent to what you have but would look neater. |
||
self.reduction_automaton.states[state].add_transition(letter, next) | ||
break | ||
next = next.subword(1, len(next)) | ||
break | ||
|
||
# Add transitions for new states. All symbols used in the automaton are considered here. | ||
# Ignore this if `generators` = `automaton_alphabet`. | ||
if len(generators) != len(automaton_alphabet): | ||
for state in proper_prefixes: | ||
current_state_name = state | ||
for letter in generators: | ||
next = current_state_name*letter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of the tests in the build fails with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is because of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it's not because of |
||
len_next_word = len(next) | ||
while True: | ||
if len(next) <= 1: | ||
if next in self.reduction_automaton.states: | ||
self.reduction_automaton.states[state].add_transition(letter, next) | ||
else: | ||
self.reduction_automaton.states[state].add_transition(letter, 'start') | ||
break | ||
else: | ||
if next in self.reduction_automaton.states: | ||
self.reduction_automaton.states[state].add_transition(letter, next) | ||
break | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comments as above |
||
next = next.subword(1, len(next)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I'm not wrong, I don't think there is a need to check the transitions at the previously added states. For example, if the left-hand side of the newly added rule is So, I think it would suffice if we just add the transitions to the newly added states and modify the transitions of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that's quite the case. What if you already have a state There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, Thank you. Makes sense. |
||
|
||
def reduce_using_automaton(self, word): | ||
''' | ||
Reduce a word using an automaton. | ||
|
||
Summary: | ||
All the symbols of the word are stored in an array and are given as the input to the automaton. | ||
If the automaton reaches a dead state that subword is replaced and the automaton is run from the beginning. | ||
The complete word has to be replaced when the word is read and the automaton reaches a dead state. | ||
So, this process is repeated until the word is read completely and the automaton reaches the accept state. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is much clearer 👍 |
||
|
||
Arguments: | ||
word (instance of FreeGroupElement) -- Word that needs to be reduced. | ||
|
||
''' | ||
# Modify the automaton if new rules are found. | ||
if self._new_rules: | ||
self._add_to_automaton(self._new_rules) | ||
self._new_rules = {} | ||
|
||
flag = 1 | ||
while flag: | ||
flag = 0 | ||
current_state = self.reduction_automaton.states['start'] | ||
word_array = [s for s in word.letter_form_elm] | ||
for i in range (0, len(word_array)): | ||
next_state_name = current_state.transitions[word_array[i]] | ||
next_state = self.reduction_automaton.states[next_state_name] | ||
if next_state.state_type == "dead": | ||
subst = next_state.rh_rule | ||
word = word.substituted_word(i - len(next_state_name) + 1, i+1, subst) | ||
flag = 1 | ||
break | ||
current_state = next_state | ||
return word |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
class State(object): | ||
''' | ||
A representation of a state managed by a ``StateMachine``. | ||
|
||
Attributes: | ||
name (instance of FreeGroupElement or string) -- State name which is also assigned to the Machine. | ||
transisitons (OrderedDict) -- Represents all the transitions of the state object. | ||
state_type (string) -- Denotes the type (accept/start/dead) of the state. | ||
rh_rule (instance of FreeGroupElement) -- right hand rule for dead state. | ||
''' | ||
|
||
def __init__(self, name, state_type=None, rh_rule=None): | ||
self.name = name | ||
self.transitions = {} | ||
self.state_type = state_type | ||
self.rh_rule = rh_rule | ||
if self.state_type == 's': | ||
self.state_type = "start" | ||
elif self.state_type == 'a': | ||
self.state_type = "accept" | ||
elif self.state_type == 'd': | ||
self.state_type = "dead" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes more sense to favor the shorter representation. So the user may enter "start" but we should be storing it as just "s". In fact, you can just say Also, probably worth checking if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
But I thought that when someone tries to understand the working of the code, the lines like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's going to be less readable with just the first letter. You could always add a comment along the lines of |
||
|
||
def add_transition(self, letter, state): | ||
''' | ||
Add a transition from the current state to a new state. | ||
|
||
Keyword Arguments: | ||
letter -- The alphabet element the current state reads to make the state transition. | ||
state -- This will be an instance of the State object which represents a new state after in the transition after the alphabet is read. | ||
|
||
''' | ||
self.transitions[letter] = state | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel that a state should have a state machine associated with it, because it doesn't make sense to have a state without a machine. Maybe as a |
||
|
||
class StateMachine(object): | ||
''' | ||
Representation of a finite state machine the manages the states and the transitions of the automaton. | ||
|
||
Attributes: | ||
states (dictionary) -- Collection of all registered `State` objects. | ||
name (str) -- Name of the state machine. | ||
''' | ||
|
||
def __init__(self, name): | ||
self.name = name | ||
self.states = {} # Contains all the states in the machine. | ||
self.add_state('start', state_type='s') | ||
|
||
def add_state(self, state_name, state_type=None, rh_rule=None): | ||
''' | ||
Instantiate a state object and stores it in the 'states' dictionary. | ||
|
||
Arguments: | ||
state_name (instance of FreeGroupElement or string) -- name of the new states. | ||
state_type (string) -- Denotes the type (accept/start/dead) of the state added. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think elaborating a bit to say that it's 'accept'/'start'/'dead' or 'a'/'s'/'d' respectively could be helpful to the user. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay 👍 |
||
rh_rule (instance of FreeGroupElement) -- right hand rule for dead state. | ||
|
||
''' | ||
new_state = State(state_name, state_type, rh_rule) | ||
self.states[state_name] = new_state | ||
|
||
def __repr__(self): | ||
return "%s" % (self.name) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,10 @@ def test_rewriting(): | |
assert G.reduce(b**3*a**4*b**-2*a) == a**5*b | ||
assert G.equals(b**2*a**-1*b, b**4*a**-1*b**-1) | ||
|
||
assert R.reduce_using_automaton(b*a*a**2*b**-1) == a**3 | ||
assert R.reduce_using_automaton(b**3*a**4*b**-2*a) == a**5*b | ||
assert R.reduce_using_automaton(b**-1*a) == a*b**-1 | ||
|
||
G = FpGroup(F, [a**3, b**3, (a*b)**2]) | ||
R = G._rewriting_system | ||
R.make_confluent() | ||
|
@@ -21,3 +25,16 @@ def test_rewriting(): | |
# but also the system should actually be confluent | ||
assert R._check_confluence() | ||
assert G.reduce(b*a**-1*b**-1*a**3*b**4*a**-1*b**-15) == a**-1*b**-1 | ||
# check for automaton reduction | ||
assert R.reduce_using_automaton(b*a**-1*b**-1*a**3*b**4*a**-1*b**-15) == a**-1*b**-1 | ||
|
||
G = FpGroup(F, [a**2, b**3, (a*b)**4]) | ||
R = G._rewriting_system | ||
assert G.reduce(a**2*b**-2*a**2*b) == b**-1 | ||
assert R.reduce_using_automaton(a**2*b**-2*a**2*b) == b**-1 | ||
assert G.reduce(a**3*b**-2*a**2*b) == a**-1*b**-1 | ||
assert R.reduce_using_automaton(a**3*b**-2*a**2*b) == a**-1*b**-1 | ||
# Check after adding a rule | ||
R.add_rule(a**2, b) | ||
assert R.reduce_using_automaton(a**2*b**-2*a**2*b) == b**-1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just noticed this: is this correct? With the rule you added, we should have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even after There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They return identity after make confluent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, this does make sense. If |
||
assert R.reduce_using_automaton(a**4*b**-2*a**2*b**3) == b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would probably be better if the name was something like
'Reduction automaton for ' + repr(G)'
whereG
is the group.