Add Tabakov-Vardi random automata generator #69
Conversation
| * Generate random NFA using Tabakov and Vardi's approach, described in the paper | ||
| * <a href="https://doi.org/10.1007/11591191_28">Experimental Evaluation of Classical Automata Constructions</a> | ||
| * by Deian Tabakov and Moshe Y. Vardi. |
There was a problem hiding this comment.
Could you maybe add this description to the class level so that the description is directly available at the overview page (like here)? You can then use a more generic description for the method (like the second generateNFAmethod). However, be careful with dots as the javadoc tool typically interprets them as the end of the description. So "Moshe Y. Vardi." probably needs to be encoded as Moshe Y Vardi.. You can check the result with mvn site.
| public static CompactNFA<Integer> generateNFA( | ||
| Random r, int size, float td, float ad, Alphabet<Integer> alphabet) { |
There was a problem hiding this comment.
Could you maybe generalize this to Alphabet<I>? You can use Alphabet#getSymbol/Alphabet#getSymbolIndex if you need to map between symbols and their respective indices. Maybe it could also be useful to add a third method that allows you to specify a MutableNFA object to write to so that you don't force CompactNFAs (like in RandomAutomata#randomDeterministic).
There was a problem hiding this comment.
Sure, easy to generalize to Alphabet<I> !
I'm not sure I understand the distinction with a MutableNFA change, or how to implement it (I tend to get lost in abstraction layers). So if you're okay with it, I'll leave that part like it is.
There was a problem hiding this comment.
Sure, I can push the changes myself afterwards.
| float ad = 0.5f; // exactly 2 accepting states | ||
| Alphabet<Integer> alphabet = Alphabets.integers(0, 1); | ||
| CompactNFA<Integer> compactNFA = TabakovVardiRandomAutomata.generateNFA(r, size, td, ad, alphabet); | ||
| Assert.assertEquals(size, compactNFA.size()); |
There was a problem hiding this comment.
The parameters of the assertEquals methods are ordered as (actual, expected). So it would be cleaner to flip the arguments for more meaningful error messages (also in the second test case).
There was a problem hiding this comment.
Thanks! Apparently I've gotten that wrong for years.
There was a problem hiding this comment.
Just FYI -- surprisingly, TestNG and JUnit use different parameter orders.
https://stackoverflow.com/questions/26102865/assertequals-what-is-actual-and-what-is-expected
| sb.append(compactNFA.getTransitions(s, 0)); | ||
| } | ||
| // 5 transitions for 0 | ||
| Assert.assertEquals("[0, 3][][1, 2][3]", sb.toString()); |
There was a problem hiding this comment.
Not a fan of String-based comparisons. Could you please compare the original Set<Integer>s instead? TestNG has assertEquals methods for sets and you can use Java 11 in the tests, so the Set.of methods are available.
Use set equality rather than String equality.
Move Javadoc into class. Fix some typos while we're here.
Cite myself.
Add Tabakov-Vardi random automata generator. This method has been used in several papers as an attempt to generate representative NFAs.
Also fix some minor Javadoc typos.