-
Notifications
You must be signed in to change notification settings - Fork 34
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
feat: Adaptive Mealy Tree Builder #56
Conversation
Dear @tiferrei, thank you for contributing. This looks like an interesting feature. However, I am a little bit conflicted about the implementation because you extend the existing Can you maybe provide some additional information about possible use-cases for this type of cache? Maybe we can then work out a separate concept/interface for these type of caches. |
Hi @mtf90, sure. So the cache is still consistent. The cache simply removes conflicting information before adding new one, instead of throwing an exception. The use case is caching learning information on systems that mutate over time, where priority is given to "fresher" information. Conflicting info is then just evidence the target system has mutated, and the previous information can no longer hold. I agree this doesn't currently meet the current interface. I'm completely open to designing a better fitting one! |
Funnily enough, your description of its behavior is exactly what I would call inconsistent 😆. Querying the same input may produce different outputs at different times. When used with the current algorithms of LearnLib this could cause problems, since most learning algorithms require a deterministic SUL behavior. Do you use a specialized learning algorithm for mutable systems with this? What I'm currently wondering about is, how one would use this class as a cache. Normally, if you have a cache miss, the query is delegated to the system and the result is inserted into the cache. Repeated queries will then never reach the SUL again. Now, if you have mutating systems, you never know if the system has mutated, so you basically have to delegate all queries to the system to potentially override old data, rendering its role of a cache useless. Also, Mealy caches are typically prefix-closed. Let's say you insert |
Hi, yes current LearnLib algorithms will not be able to handle these kind of mutating systems. We do have an algorithm for it, however we can't contribute our fork to learn lib until this cache is in place for it. In reality we use it more than solely as a cache, still one of the benefits it provides for us is still caching. To answer the last question, |
I see. I assume you plan on using some of the other methods of the Otherwise, I'm thinking about moving some interfaces around. I.e., making This way, a potential |
Hmm we do make use of the usual methods of interaction like lookup, insert, etc. but also I think the idea of the restructured interfaces is perfect! It does make things clearer, and sounds like the AdaptiveTree could live happily there |
Would you then kick off this idea by providing an interface that contains all the methods that you deem relevant? For example, I saw that you need a slightly adjusted Some remarks about the current implementation: As far as I understood, you use the Also, do you think this is a fuctionality that a generic "AdaptiveMealyBuilder" should provide or is this something specific to your learning algorithm? Would it potentially be cleaner to do the book-keeping in some utility class in the learner's module and just offer the overriding/conflict checks in the adapter builder? |
So some of these details we might actually not need. Let me clean this up a bit and I'll push a more minimal version of the interface, if we are to extract it into one. I believe that at its core, the Adaptive version should be like the classic incremental builder, with the exception of never throwing conflict exceptions. Instead it fixes these by removing conflicting information. |
Great. I'll have look at the refactoring afterwards. Just another brainstorm: Maybe it could be useful to return the sub-tree that would be / is overriden when inserting conflicting traces (either for the |
What do you think of making insert have the exact same signature (except the exception thrown) as the classic case? In theory the same use can be obtained by calling |
I'm fine with anything that leads to an elegant implementation. You know the requirements of your algorithm better than I do. I was just trying to highlight that we are not restricted by the "old" signatures. If you have no use for the replaced sub-tree, we could also return the index of the first mismatch (so it may be easier to trace the change in the automaton) or just keep the boolean value if any additional information doen't matter at all. |
For me specifically we just need to know if this new information causes a conflict or not. That being said, any of these options gives us this info, so I don't mind making them more information-heavy. |
If you're happy with a boolean return value, we should use this then. We don't have to make it overly complex just for complexity's sake. It was just an idea that maybe you could skip the |
I don't think we need the 2 separate methods. If you are ok with |
Well then that is the perfect moment to introduce the new interface and adjust the class hierarchy. I dabbled around a little bit and have a patch ready (see below). If you invite me as a collaborator, I can also push it to the repo directly. diffdiff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/AbstractIncrementalMealyBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/AbstractIncrementalMealyBuilder.java
index 0520fe710..d9f44928c 100644
--- a/incremental/src/main/java/net/automatalib/incremental/mealy/AbstractIncrementalMealyBuilder.java
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/AbstractIncrementalMealyBuilder.java
@@ -26,15 +26,15 @@ import net.automatalib.visualization.VisualizationHelper;
import net.automatalib.words.Word;
import net.automatalib.words.WordBuilder;
-public abstract class AbstractIncrementalMealyBuilder<I, O> implements IncrementalMealyBuilder<I, O> {
+public abstract class AbstractIncrementalMealyBuilder<I, O> {
+
+ public abstract boolean lookup(Word<? extends I> inputWord, List<? super O> output);
- @Override
public boolean hasDefinitiveInformation(Word<? extends I> word) {
List<O> unused = new ArrayList<>(word.length());
return lookup(word, unused);
}
- @Override
public Word<O> lookup(Word<? extends I> inputWord) {
WordBuilder<O> wb = new WordBuilder<>(inputWord.size());
lookup(inputWord, wb);
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/AdaptiveMealyBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/AdaptiveMealyBuilder.java
new file mode 100644
index 000000000..cc5e8d69d
--- /dev/null
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/AdaptiveMealyBuilder.java
@@ -0,0 +1,63 @@
+/* Copyright (C) 2013-2022 TU Dortmund
+ * This file is part of AutomataLib, http://www.automatalib.net/.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package net.automatalib.incremental.mealy;
+
+import java.util.List;
+
+import net.automatalib.SupportsGrowingAlphabet;
+import net.automatalib.automata.transducers.MealyMachine;
+import net.automatalib.incremental.IncrementalConstruction;
+import net.automatalib.ts.output.MealyTransitionSystem;
+import net.automatalib.words.Word;
+
+public interface AdaptiveMealyBuilder<I, O>
+ extends IncrementalConstruction<MealyMachine<?, I, ?, O>, I>, SupportsGrowingAlphabet<I> {
+
+ Word<O> lookup(Word<? extends I> inputWord);
+
+ /**
+ * Retrieves the output word for the given input word. If no definitive information for the input word exists, the
+ * output for the longest known prefix will be returned.
+ *
+ * @param inputWord
+ * the input word
+ * @param output
+ * a consumer for constructing the output word
+ *
+ * @return {@code true} if the information contained was complete (in this case, {@code word.length() ==
+ * output.size()} will hold), {@code false} otherwise.
+ */
+ boolean lookup(Word<? extends I> inputWord, List<? super O> output);
+
+ /**
+ * Incorporates a pair of input/output words into the stored information.
+ *
+ * @param inputWord
+ * the input word
+ * @param outputWord
+ * the corresponding output word
+ *
+ * @return {@code true} if the inserted output word has overridden existing information, {@code false} otherwise.
+ */
+ boolean insert(Word<? extends I> inputWord, Word<? extends O> outputWord);
+
+ @Override
+ GraphView<I, O, ?, ?> asGraph();
+
+ @Override
+ MealyTransitionSystem<?, I, ?, O> asTransitionSystem();
+
+}
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/GraphView.java b/incremental/src/main/java/net/automatalib/incremental/mealy/GraphView.java
new file mode 100644
index 000000000..c0c6f2c6e
--- /dev/null
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/GraphView.java
@@ -0,0 +1,27 @@
+/* Copyright (C) 2013-2022 TU Dortmund
+ * This file is part of AutomataLib, http://www.automatalib.net/.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package net.automatalib.incremental.mealy;
+
+import net.automatalib.graphs.Graph;
+
+interface GraphView<I, O, N, E> extends Graph<N, E> {
+
+ I getInputSymbol(E edge);
+
+ O getOutputSymbol(E edge);
+
+ N getInitialNode();
+}
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/IncrementalMealyBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/IncrementalMealyBuilder.java
index f1695ea35..cb695bf25 100644
--- a/incremental/src/main/java/net/automatalib/incremental/mealy/IncrementalMealyBuilder.java
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/IncrementalMealyBuilder.java
@@ -19,7 +19,6 @@ import java.util.List;
import net.automatalib.SupportsGrowingAlphabet;
import net.automatalib.automata.transducers.MealyMachine;
-import net.automatalib.graphs.Graph;
import net.automatalib.incremental.ConflictException;
import net.automatalib.incremental.IncrementalConstruction;
import net.automatalib.ts.output.MealyTransitionSystem;
@@ -63,12 +62,4 @@ public interface IncrementalMealyBuilder<I, O>
@Override
MealyTransitionSystem<?, I, ?, O> asTransitionSystem();
- interface GraphView<I, O, N, E> extends Graph<N, E> {
-
- I getInputSymbol(E edge);
-
- O getOutputSymbol(E edge);
-
- N getInitialNode();
- }
}
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/dag/IncrementalMealyDAGBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/dag/IncrementalMealyDAGBuilder.java
index 7dad11bba..2515170bc 100644
--- a/incremental/src/main/java/net/automatalib/incremental/mealy/dag/IncrementalMealyDAGBuilder.java
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/dag/IncrementalMealyDAGBuilder.java
@@ -34,6 +34,7 @@ import net.automatalib.commons.util.IntDisjointSets;
import net.automatalib.commons.util.UnionFind;
import net.automatalib.incremental.ConflictException;
import net.automatalib.incremental.mealy.AbstractIncrementalMealyBuilder;
+import net.automatalib.incremental.mealy.IncrementalMealyBuilder;
import net.automatalib.ts.output.MealyTransitionSystem;
import net.automatalib.visualization.VisualizationHelper;
import net.automatalib.visualization.helper.DelegateVisualizationHelper;
@@ -54,7 +55,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
* @author Malte Isberner
*/
public class IncrementalMealyDAGBuilder<I, O> extends AbstractIncrementalMealyBuilder<I, O>
- implements InputAlphabetHolder<I> {
+ implements IncrementalMealyBuilder<I, O>, InputAlphabetHolder<I> {
private final Map<@Nullable StateSignature<O>, State<O>> register = new HashMap<>();
private final Alphabet<I> inputAlphabet;
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AbstractIncrementalMealyTreeBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AbstractIncrementalMealyTreeBuilder.java
index bc0fb4660..df9bef539 100644
--- a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AbstractIncrementalMealyTreeBuilder.java
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AbstractIncrementalMealyTreeBuilder.java
@@ -46,7 +46,6 @@ public abstract class AbstractIncrementalMealyTreeBuilder<N, I, O> extends Abstr
this.root = root;
}
- @Override
public boolean lookup(Word<? extends I> word, List<? super O> output) {
N curr = root;
@@ -62,7 +61,6 @@ public abstract class AbstractIncrementalMealyTreeBuilder<N, I, O> extends Abstr
return true;
}
- @Override
public void insert(Word<? extends I> input, Word<? extends O> outputWord) {
N curr = root;
@@ -81,17 +79,14 @@ public abstract class AbstractIncrementalMealyTreeBuilder<N, I, O> extends Abstr
}
}
- @Override
public GraphView asGraph() {
return new GraphView();
}
- @Override
public TransitionSystemView asTransitionSystem() {
return new TransitionSystemView();
}
- @Override
public @Nullable Word<I> findSeparatingWord(MealyMachine<?, I, ?, O> target,
Collection<? extends I> inputs,
boolean omitUndefined) {
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AdaptiveMealyTreeBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AdaptiveMealyTreeBuilder.java
index aa7706cb0..7d54d6d2e 100644
--- a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AdaptiveMealyTreeBuilder.java
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/AdaptiveMealyTreeBuilder.java
@@ -15,27 +15,99 @@
*/
package net.automatalib.incremental.mealy.tree;
+import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.PriorityQueue;
+import net.automatalib.automata.concepts.InputAlphabetHolder;
import net.automatalib.commons.util.Pair;
+import net.automatalib.incremental.mealy.IncrementalMealyBuilder;
import net.automatalib.util.graphs.traversal.GraphTraversal;
import net.automatalib.words.Alphabet;
import net.automatalib.words.Word;
+import net.automatalib.words.impl.Alphabets;
+
+public class AdaptiveMealyTreeBuilder<I, O> extends AbstractIncrementalMealyTreeBuilder<Node<O>, I, O>
+ implements IncrementalMealyBuilder<I, O>, InputAlphabetHolder<I> {
-public class AdaptiveMealyTreeBuilder<I, O> extends IncrementalMealyTreeBuilder<I, O> {
private final Map<Node<O>, Pair<Word<? extends I>, Integer>> stateToQuery = new HashMap<>();
private final PriorityQueue<Node<O>> queryStates = new PriorityQueue<>(
(Node<O> a, Node<O> b) -> Integer.compare(stateToQuery.get(a).getSecond(),
stateToQuery.get(b).getSecond()));
private Integer insertCounter = 0;
+ private final Alphabet<I> inputAlphabet;
+ private int alphabetSize;
+
public AdaptiveMealyTreeBuilder(Alphabet<I> inputAlphabet) {
- super(inputAlphabet);
+ super(new Node<>(inputAlphabet.size()));
+ this.inputAlphabet = inputAlphabet;
+ this.alphabetSize = inputAlphabet.size();
+ }
+
+ @Override
+ public void addAlphabetSymbol(I symbol) {
+ if (!inputAlphabet.containsSymbol(symbol)) {
+ Alphabets.toGrowingAlphabetOrThrowException(inputAlphabet).addSymbol(symbol);
+ }
+
+ final int newAlphabetSize = inputAlphabet.size();
+ // even if the symbol was already in the alphabet, we need to make sure to be able to store the new symbol
+ if (alphabetSize < newAlphabetSize) {
+ ensureInputCapacity(root, alphabetSize, newAlphabetSize);
+ alphabetSize = newAlphabetSize;
+ }
+ }
+
+ private void ensureInputCapacity(Node<O> node, int oldAlphabetSize, int newAlphabetSize) {
+ node.ensureInputCapacity(newAlphabetSize);
+ for (int i = 0; i < oldAlphabetSize; i++) {
+ final Node<O> child = node.getSuccessor(i);
+ if (child != null) {
+ ensureInputCapacity(child, oldAlphabetSize, newAlphabetSize);
+ }
+ }
+ }
+
+ @Override
+ protected Edge<Node<O>, O> getEdge(Node<O> node, I symbol) {
+ return node.getEdge(inputAlphabet.getSymbolIndex(symbol));
+ }
+
+ @Override
+ protected Node<O> createNode() {
+ return new Node<>(alphabetSize);
+ }
+
+ @Override
+ protected Node<O> insertNode(Node<O> parent, I symIdx, O output) {
+ Node<O> succ = createNode();
+ Edge<Node<O>, O> edge = new Edge<>(output, succ);
+ parent.setEdge(inputAlphabet.getSymbolIndex(symIdx), edge);
+ return succ;
+ }
+
+ @Override
+ protected Collection<AnnotatedEdge<Node<O>, I, O>> getOutgoingEdges(Node<O> node) {
+ List<AnnotatedEdge<Node<O>, I, O>> result = new ArrayList<>(alphabetSize);
+ for (int i = 0; i < alphabetSize; i++) {
+ Edge<Node<O>, O> edge = node.getEdge(i);
+ if (edge != null) {
+ result.add(new AnnotatedEdge<>(edge, inputAlphabet.getSymbol(i)));
+ }
+ }
+ return result;
+ }
+
+ @Override
+ public Alphabet<I> getInputAlphabet() {
+ return inputAlphabet;
}
@Override
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/IncrementalMealyTreeBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/IncrementalMealyTreeBuilder.java
index 4486d6afd..228d366d5 100644
--- a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/IncrementalMealyTreeBuilder.java
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/IncrementalMealyTreeBuilder.java
@@ -20,11 +20,12 @@ import java.util.Collection;
import java.util.List;
import net.automatalib.automata.concepts.InputAlphabetHolder;
+import net.automatalib.incremental.mealy.IncrementalMealyBuilder;
import net.automatalib.words.Alphabet;
import net.automatalib.words.impl.Alphabets;
public class IncrementalMealyTreeBuilder<I, O> extends AbstractIncrementalMealyTreeBuilder<Node<O>, I, O>
- implements InputAlphabetHolder<I> {
+ implements IncrementalMealyBuilder<I, O>, InputAlphabetHolder<I> {
private final Alphabet<I> inputAlphabet;
private int alphabetSize;
diff --git a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/dynamic/DynamicIncrementalMealyTreeBuilder.java b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/dynamic/DynamicIncrementalMealyTreeBuilder.java
index 0fc834443..8535ba224 100644
--- a/incremental/src/main/java/net/automatalib/incremental/mealy/tree/dynamic/DynamicIncrementalMealyTreeBuilder.java
+++ b/incremental/src/main/java/net/automatalib/incremental/mealy/tree/dynamic/DynamicIncrementalMealyTreeBuilder.java
@@ -20,6 +20,7 @@ import java.util.Collection;
import java.util.List;
import java.util.Map;
+import net.automatalib.incremental.mealy.IncrementalMealyBuilder;
import net.automatalib.incremental.mealy.tree.AbstractIncrementalMealyTreeBuilder;
import net.automatalib.incremental.mealy.tree.AnnotatedEdge;
import net.automatalib.incremental.mealy.tree.Edge;
@@ -41,7 +42,8 @@ import org.checkerframework.checker.nullness.qual.Nullable;
*
* @author frohme
*/
-public class DynamicIncrementalMealyTreeBuilder<I, O> extends AbstractIncrementalMealyTreeBuilder<Node<I, O>, I, O> {
+public class DynamicIncrementalMealyTreeBuilder<I, O> extends AbstractIncrementalMealyTreeBuilder<Node<I, O>, I, O>
+ implements IncrementalMealyBuilder<I, O> {
public DynamicIncrementalMealyTreeBuilder() {
this(new Node<>()); |
Sounds great, I have added you! |
You should now be able to adjust the The only (potentially breaking) change that I have introduced so far is that the insert method now expects an output If you are happy and adjusted everything according to your needs, I can have another look at some more cleanup work (extracting shared functionality, making code-analysis pass, etc.) |
Ah, hmm. I believe I can't currently extend |
Sorry, I didn't think the last step through. I added some of the cleanup stuff already (mainly an intermediate abstraction layer to share some more functionality). It should be easier now to swtich to the adaptive interface. |
I have pushed the switch! |
This reverts commit 2ba1ae0.
Great! So the next step would be to check if you need any additional methods for your algorithm or if you even can remove some unnecessary stuff (if I recall correctly, your book-keeping of oldest queries was somewhat optional?). Given the new Let me know if you feel that all requirements are met, so that I'll have a final look at cleaning up before merging. If in the meantime you have any questions, feel free to ask. |
So for our specific algorithm we do make use of retrieving the oldest query info. I have removed the info from the method signature and into an internal counter, as in our case we can assume these are strictly increasing, where the most recently inserted info is the freshest. In that sense, if we were to, for example, make a |
Maybe |
|
Ah yes I see. In that case I believe it is done, requirements wise. Let me know if you'd like me to do any further changes, and thank you for the help! |
I refactored some of the previous cleanups. Mainly, this was just moving the alphabet related code into a shared super class. We have to do some code copying because java doesn't support multiple inheritance. But I rather copy just the Furthermore, I did refactor some parts of the adaptive implementation:
If you have nothing more to add, I'm happy with merging. |
This sounds fantastic! Indeed the tests cover my intended behaviour, and I also just compiled and tested things locally on the algorithms and all runs smoothly. I have nothing more to add and happy with merging too. |
Hi there,
In a recent paper to be published we make use of a type of Tree that allows for conflicts. As such I'd like to contribute our extension of Incremental Mealy Tree Builders.
Please let me know if there are any design choices, improvements, or questions you may have to merge this in.
Thank you!
Tiago