coli-saar / alto

Alto, the Algebraic Language Toolkit
Other
16 stars 2 forks source link

Clean up InverseHomAutomaton #70

Open alexanderkoller opened 3 years ago

alexanderkoller commented 3 years ago

The class InverseHomAutomaton can compute the inverse homomorphism of a tree automaton under an arbitrary tree homomorphism. Because almost all homomorphisms we use in applications of Alto are non-deleting and there is a better maintained class NondeletingInverseHomAutomaton, the code in this class is badly maintained and probably has bugs.

We should clean up the code and write unit tests. The source code has several comments that mention possible bugs.

It is also fishy that InverseHomAutomaton<State> extends TreeAutomaton<Object>; this is due to the presence of a failure state, which is an object of class String. It would probably be cleaner to extend TreeAutomaton<Optional<State>> and use Optional.empty() as the failure state.

The comparison of states to the failure state are fishy too. In this snippet, an int (parentState) is compared to a string:

public Iterable<Rule> getRulesTopDown(int label, int parentState) {
        if (FAIL_STATE.equals(parentState)) {

And this code seems unreliable too:

isFailedRule(Rule rule, TreeAutomaton auto) {
     if (auto.getStateForId(rule.getParent()).toString().contains(InverseHomAutomaton.FAIL_STATE) {
        return true;
akoehn commented 3 years ago

My current idea is to write a warning to stderr the first time an InverseHomAutomaton is created, notifying the user of possible bugs. Due to the way automata are selected in alto, it might not always be obvious which implementation is actually used.