Fundamental Refactoring of SL Lambda Learning Algorithm #116
Conversation
fhowar
left a comment
fhowar
left a comment
There was a problem hiding this comment.
This looks amazing! The implementation is greatly improved. I only ask for small name changes / documentation and removal of deprecated methods.
For the prefix finder: will it be able to search with binary search?
There was a problem hiding this comment.
Is it possible to remove the deprecated method?
There was a problem hiding this comment.
+1 for removal. Esp. given the comment above it that it is unsafe.
There was a problem hiding this comment.
The reason why these methods are deprecated is that the XML automata models make use of the unsafe functionality, in that they are coded to only assign a parameter to a register. They do not reassign old values for registers that will be reused. Because of this, removing the deprecated methods will require refactoring of the XML models. I am not opposed to refactoring the models, but I feel like this is beyond the scope of this PR and should perhaps be done in a separate PR.
There was a problem hiding this comment.
Could you add that to the method's documentation?
There was a problem hiding this comment.
It's reasonable to leave this for another PR. However, so as to not forget about it, besides adding this note to the method's documentation, it's good to also open a GitHub issue about this refactoring.
There was a problem hiding this comment.
can we remove the deprecated method?
There was a problem hiding this comment.
Could you add some documentation or find a better name for tryEquality
There was a problem hiding this comment.
Since this is a boolean method, something starting with is is an appropriate name.
Suggestions: something like isEqualitySolvable or isSolvableByEquality ?
| return Optional.empty(); | ||
| } | ||
|
|
||
| private boolean tryEquality(Expression<Boolean> guard, Parameter p, DataValue val, ConstraintSolver solver) { |
The prefix finder currently searches from right to left. However, each loop iteration is independent, so it would be straightforward to implement a binary search. |
|
The code of this PR now has various javadoc problems that probably have been exposed by PR #114 that added javadoc support. These need to be fixed before this PR is merged. |
This PR completely reworks the implementation for SL lambda to align it more closely with the SL lambda algorithm. The new implementation aims to be more modular in order to facilitate future additions and expansions to the SL lambda algorithm. By aligning the implementation more closely with the algorithm, this PR also fixes issues involving non-determinacy during learning, such as non-determinacy problems revealed when examining issue #78 (which is fixed with this PR). This PR also fixes a number of bugs in ralib.
Among the contributions made by this PR are the following: