Skip to content
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

:no-loop loops when logical inserts invalidate their own LHS constraints #99

Closed
mrrodriguez opened this issue Mar 2, 2015 · 6 comments
Milestone

Comments

@mrrodriguez
Copy link
Collaborator

I am not positive on this one, but I was expecting the :no-loop attribute on a rule to stop a logical loop from happening in the following rule:

(let [looper (dsl/parse-rule [[:not [:marker]]]

                             (insert! ^{:type :marker} {})
                             {:no-loop true})
      s (mk-session [looper])]

  (fire-rules s))

However, it looks to go into an infinite loop when I run it.

Is this an edge-case that is not being caught by the :no-loop logic implemented? This differs from the existing clara.test-rules tests around the :no-loop in that the loop is a result of a logical insert! having its logical-truth in the LHS becoming false due to the insert itself.

I haven't compared this with engines such as Drools yet to see if this use-case is not an intended way to use the :no-loop property.

Also, I know that a situation like this can be worked around via adding another rule and having some "guards"/"blocker" facts involved.

@rbrush
Copy link
Contributor

rbrush commented Mar 2, 2015

The :no-loop directive will prevent a fact inserted by a rule from triggering a re-activation, but it (currently) won't prevent truth maintenance from doing so, which is what you're likely seeing here.

:no-loop and truth maintenance seem like they are almost at adds, honestly...since truth maintenance is designed to keep the session in a consistent logical state and :no-loop artificially blocks certain rules from firing. (I actually was reluctant to add :no-loop to begin with because it's artificial.)

In any case, the current behavior for :no-loop and truth maintenance would likely surprise a lot of users, so we should do something about it. I think our options are either to 1.) explicitly prevent logical inserts in :no-loop rules, or to 2.) modify the truth maintenance system to handle this flow.

I'd lean towards 2, but I want to make sure we understand the implications. If we can't come up with a clear, compelling "of course it should work that way!" behavior when combining :no-loop and truth maintenance for all scenarios, then disallowing it would be appropriate to prevent surprising behavior that can cause bugs.

@mrrodriguez
Copy link
Collaborator Author

I agree that :no-loop seems to be mostly undesirable. I had a case where I could see :no-loop adding some concision for a rule with an :or where I didn't want it to be able to fire and insert the same fact twice.

However, this can be worked around by being a bit more verbose and just writing out another rule to deal with it. I think in general something like this can always be done.

I mostly raised this issue out of surprise at how it behaved with truth maintenance. It does seem to be a strange concept to think about that a :no-loop would allow for a logical insert to potentially not logically supported at some state of working memory. That doesn't necessary sit right. Perhaps the solution of (1) is the best to just not allow the weird semantics.

rbrush added a commit that referenced this issue Mar 6, 2015
@rbrush
Copy link
Contributor

rbrush commented Mar 6, 2015

So this turned out to be cleaner than I thought...it's just a matter of apply the :no-loop rule for truth maintenance retractions in a way that's completely symmetric to activations. Like :no-loop for rule activations, it only applies when the change is triggered directly by the rule marked as :no-loop.

I went ahead and fixed it in the above commit. I'm still not a huge fan of :no-loop, but it's nice for instances like this and we should behave intuitively.

@rbrush rbrush added this to the 0.8.6 milestone Mar 6, 2015
@rbrush
Copy link
Contributor

rbrush commented Mar 6, 2015

Adding to the 0.8.6 milestone, which I'd like to push out this weekend unless there are objections to the behavior described above.

@mrrodriguez
Copy link
Collaborator Author

Thanks for the update and quick response. It looks like a simple change to make it work too.

@rbrush
Copy link
Contributor

rbrush commented Mar 8, 2015

Yeah, the fact that it's a clean symmetry for no-loop rules between fact assertions and retractions made me feel like this is the right approach for this particular case.

I'm closing this now with the intention of including the change in the 0.8.6 release.

@rbrush rbrush closed this as completed Mar 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants