Refactor DefaultSolver #297
Conversation
- DefaultSolver#tryAdvance broken into smaller methods, added SearchState member. - Small polishing in PerformanceLog.
- Move writing methods to WritableAssignment.
- WritableAssignment and TrailAssignment realize didChange() method. - DefaultSolver uses WritableAssignment#didChange, plus minor polishing.
| return; | ||
| } | ||
| // We already found one Answer-Set and are requested to find another one. | ||
| searchState.afterAllAtomsAssigned = false; |
There was a problem hiding this comment.
If I read the code correctly, we reach this part of the code on the second (and subsequent) call(s) to tryAdvance (as in the first call, we'll return after initializing search space). This is not really intuitive from the comment here, I think readability would benefit from a somewhat more detailed explanation on why we only get there after finding an answer set in the comment.
There was a problem hiding this comment.
Yes, the code is doing that. I split up the method into two separate ones.
Co-authored-by: Lorenz Leutgeb <lorenz@leutgeb.xyz>
Codecov Report
@@ Coverage Diff @@
## master #297 +/- ##
============================================
+ Coverage 78.35% 78.48% +0.13%
+ Complexity 2659 2515 -144
============================================
Files 191 189 -2
Lines 8870 8199 -671
Branches 1592 1417 -175
============================================
- Hits 6950 6435 -515
+ Misses 1449 1324 -125
+ Partials 471 440 -31
Continue to review full report at Codecov.
|
- Choice provides getInverted now, renamed some variables. - ChoiceManager refactored, provides only one backtrack method now, not two semi-duplicates. - Backtracking code in DefaultSolver streamlined.
|
@lorenzleutgeb and/or @madmike200590 please have another look. |
Some refactoring/polishing inside the solver component, mostly DefaultSolver. Changes are intended to support subclassing of DefaultSolver for optimization tasks/weak constraints.