Uploaded image for project: 'OptaPlanner'
  1. OptaPlanner
  2. PLANNER-1433

ConstraintMatchTotal does not handle 'or' rule situations properly

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Won't Do
    • Icon: Critical Critical
    • None
    • None
    • optaplanner-core
    • None
    • 2019 Week 08-10, 2019 Week 11-13
    • 3
    • NEW
    • NEW

      ConstraintMatchTotal throws an exception during use of reasonable rules that have an 'or' condition whose terms match the same class type while looking at different attributes.

      For example, suppose the following snippet is part of a .drl rule, where unique BoundaryDate objects are defined by a dayIndex attribute. If it happens during solving that $firstDayIndex equals $lastDayIndex (due to earlier matching in the rule, not shown), and the BoundaryDate at that single value happens to have its other flags preferredSequenceStart and preferredSequenceEnd attributes false, then drools is matching this rule twice, and sending the identical justificationList to ConstraintMatchTotal.addConstraintMatch, which then throws an exception.

      (            
              BoundaryDate(
              	dayIndex == $firstDayIndex,
              	preferredSequenceStart == false)					// work sequence does NOT start on a boundary
                or													// or...
              BoundaryDate(
              	dayIndex == $lastDayIndex,
              	preferredSequenceEnd == false)						// work sequence does NOT end on a boundary
              )
      

      The exception is thrown below:

      public ConstraintMatch addConstraintMatch(List<Object> justificationList, Score score) {
              this.score = this.score.add(score);
              ConstraintMatch constraintMatch = new ConstraintMatch(constraintPackage, constraintName,
                      justificationList, score);
              boolean added = constraintMatchSet.add(constraintMatch);
              if (!added) {
                  *throw new IllegalStateException*("The constraintMatchTotal (" + this
                          + ") could not add constraintMatch (" + constraintMatch
                          + ") to its constraintMatchSet (" + constraintMatchSet + ").");
              }
              return constraintMatch;
          }
       
          public void removeConstraintMatch(ConstraintMatch constraintMatch) {
              score = score.subtract(constraintMatch.getScore());
              boolean removed = constraintMatchSet.remove(constraintMatch);
              if (!removed) {
                  throw new IllegalStateException("The constraintMatchTotal (" + this
                          + ") could not remove constraintMatch (" + constraintMatch
                          + ") from its constraintMatchSet (" + constraintMatchSet + ").");
              }
          }
      

      A proposed fix is to replace this code with:

      public ConstraintMatch addConstraintMatch(List<Object> justificationList, Score score) {
              ConstraintMatch constraintMatch = new ConstraintMatch(constraintPackage, constraintName,
                      justificationList, score);
              boolean added = constraintMatchSet.add(constraintMatch);
              if (added) {
                  this.score = this.score.add(score);    // confirmed a distinct justification list addition (it is possible for multiple calls to come from rules with 'or' conditions)
              }
              return constraintMatch;
          }
       
          public void removeConstraintMatch(ConstraintMatch constraintMatch) {
              boolean removed = constraintMatchSet.remove(constraintMatch);
              if (removed) {
                  score = score.subtract(constraintMatch.getScore());    // confirmed a distinct justification list removal
              }
          }
      

      This may be the complete root issue for PLANNER-1218, and I see other open reports of IllegalStateExceptions thrown by constraintMatchTotal that might be a result of this as well.

              lpetrovi@redhat.com Lukáš Petrovický (Inactive)
              frabla W M (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Created:
                Updated:
                Resolved: