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

ProductionNode removes a pending activation when a retraction corresponds to a previously-fired activation from an equal token #321

Open
WilliamParker opened this Issue Jun 25, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@WilliamParker
Member

WilliamParker commented Jun 25, 2017

clara.test-rules> (defrule demo-rule [First] [?s <- Second] => (retract! ?s))
#'clara.test-rules/demo-rule
clara.test-rules> (defquery get-second [] [?s <- Second])
#'clara.test-rules/get-second
clara.test-rules> (def empty-session (mk-session [demo-rule get-second] :cache false))
#'clara.test-rules/empty-session
clara.test-rules> (-> empty-session (insert (->First) (->Second) (->Second)) fire-rules (query get-second))
({:?s {}})

I would expect to not get any Second facts from the query after firing the rules in this session, but instead I get 1 Second fact. I believe this is related to the behavior Jerry Jackson reported on the mailing list:

Also, I don't understand how this rule:

(defrule one-service
  ""
  {:salience 2000}
  [?svc1 <- :service [{id :id}] (= ?id1 id)]
  [?svc2 <- :service [{id :id}] (= ?id2 id)]
  [:test (= ?id1 ?id2)]
  =>
  (retract! ?svc2))

can result in more than one service with the same id remaining in working memory. Why doesn't it continue to fire until there is only one left?

Again, thanks for any help.

I created a more simplified example that eliminates the behavior of facts joining with themselves. In this example, I believe what happens is as follows, with inline links to relevant places in the code.

As a result, demo-rule only fires once and only one Second fact is retracted.

@WilliamParker WilliamParker added the bug label Jun 25, 2017

@mrrodriguez

This comment has been minimized.

Collaborator

mrrodriguez commented Jun 26, 2017

This seems related to the old and still present issue #229

It just seems like things get complicated and easily out of sync when RHS retracts are performed "eagerly", while inserts are batched. I've seen other bad behaviors around this before. I don't think it makes sense for it to behave the way it does either (well obviously not given the logical incorrectness presented here).

Just pointing this out to keep it in mind.

@WilliamParker

This comment has been minimized.

Member

WilliamParker commented Jun 26, 2017

Agreed, this seems related to #229. I think it might be possible to use looping rules without RHS retracts to expose this behavior, but at this time I don't have a concrete example. It seems to me that there is some ambiguity in the ProductionNode's behavior - supposing that a given token has both a pending activation and a previous activation, which does it make sense to remove when that token is retracted? Since Clara has value-based, not identity-based, semantics around facts I'm not sure there is a definitive answer to that question; perhaps this shouldn't be considered a bug and should just be determined to be undefined behavior. Longer-term it would be good to determine and document exactly what behavior is defined and what is undefined in this area though.

@mrrodriguez

This comment has been minimized.

Collaborator

mrrodriguez commented Jun 29, 2017

Without a ton of analysis on it I think I understand the ambiguity you're talking about. It'd be interesting to have a case for it that isn't relying on RHS retract and that showed that behavior could be unexpected depending on if the pending or previous activation was the one retracted.

Saying it is undefined may be confusing because it might be hard to really understand what would cause this.

The value-based removal can certainly make some cases hard to deal with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment